From 7d5cb9de49e496ef073f082d7206097292f5e9a6 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Tue, 12 Nov 2024 04:35:15 -0500 Subject: [PATCH 01/12] Cleanup URL tracking tests. (#5181) --- crates/utils/src/utils/validation.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index f8da6f609..4863b11b8 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -379,11 +379,14 @@ mod tests { use pretty_assertions::assert_eq; use url::Url; + const URL_WITH_TRACKING: &str = "https://example.com/path/123?utm_content=buffercf3b2&utm_medium=social&user+name=random+user&id=123"; + const URL_TRACKING_REMOVED: &str = "https://example.com/path/123?user+name=random+user&id=123"; + #[test] fn test_clean_url_params() -> LemmyResult<()> { - let url = Url::parse("https://example.com/path/123?utm_content=buffercf3b2&utm_medium=social&user+name=random+user&id=123")?; + let url = Url::parse(URL_WITH_TRACKING)?; let cleaned = clean_url(&url); - let expected = Url::parse("https://example.com/path/123?user+name=random+user&id=123")?; + let expected = Url::parse(URL_TRACKING_REMOVED)?; assert_eq!(expected.to_string(), cleaned.to_string()); let url = Url::parse("https://example.com/path/123")?; @@ -395,9 +398,9 @@ mod tests { #[test] fn test_clean_body() -> LemmyResult<()> { - let text = "[a link](https://example.com/path/123?utm_content=buffercf3b2&utm_medium=social&user+name=random+user&id=123)"; - let cleaned = clean_urls_in_text(text); - let expected = "[a link](https://example.com/path/123?user+name=random+user&id=123)"; + let text = format!("[a link]({URL_WITH_TRACKING})"); + let cleaned = clean_urls_in_text(&text); + let expected = format!("[a link]({URL_TRACKING_REMOVED})"); assert_eq!(expected.to_string(), cleaned.to_string()); let text = "[a link](https://example.com/path/123)"; From a8fb55d6c833f99e8b3907ac7527d9abe84f1b79 Mon Sep 17 00:00:00 2001 From: SleeplessOne1917 <28871516+SleeplessOne1917@users.noreply.github.com> Date: Tue, 12 Nov 2024 10:54:23 +0000 Subject: [PATCH 02/12] Simplify regex for user/community name validation (#5164) * Add lowercase only check for community names * Lint * Remove redundant check * Lint * Add newline escape tests * Eliminate confounding factor in community name test * Use same check for user names and community names * Use min/max length check --------- Co-authored-by: Dessalines Co-authored-by: Felix Ableitner --- crates/utils/src/utils/validation.rs | 32 +++++++++++++++------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index 4863b11b8..43aae8599 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -85,26 +85,19 @@ fn has_newline(name: &str) -> bool { } pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> LemmyResult<()> { - static VALID_ACTOR_NAME_REGEX_EN: LazyLock = - LazyLock::new(|| Regex::new(r"^[a-zA-Z0-9_]{3,}$").expect("compile regex")); - static VALID_ACTOR_NAME_REGEX_AR: LazyLock = - LazyLock::new(|| Regex::new(r"^[\p{Arabic}0-9_]{3,}$").expect("compile regex")); - static VALID_ACTOR_NAME_REGEX_RU: LazyLock = - LazyLock::new(|| Regex::new(r"^[\p{Cyrillic}0-9_]{3,}$").expect("compile regex")); - - let check = name.chars().count() <= actor_name_max_length && !has_newline(name); - // Only allow characters from a single alphabet per username. This avoids problems with lookalike // characters like `o` which looks identical in Latin and Cyrillic, and can be used to imitate // other users. Checks for additional alphabets can be added in the same way. - let lang_check = VALID_ACTOR_NAME_REGEX_EN.is_match(name) - || VALID_ACTOR_NAME_REGEX_AR.is_match(name) - || VALID_ACTOR_NAME_REGEX_RU.is_match(name); + static VALID_ACTOR_NAME_REGEX: LazyLock = LazyLock::new(|| { + Regex::new(r"^(?:[a-zA-Z0-9_]+|[0-9_\p{Arabic}]+|[0-9_\p{Cyrillic}]+)$").expect("compile regex") + }); - if !check || !lang_check { - Err(LemmyErrorType::InvalidName.into()) - } else { + min_length_check(name, 3, LemmyErrorType::InvalidName)?; + max_length_check(name, actor_name_max_length, LemmyErrorType::InvalidName)?; + if VALID_ACTOR_NAME_REGEX.is_match(name) { Ok(()) + } else { + Err(LemmyErrorType::InvalidName.into()) } } @@ -441,6 +434,15 @@ mod tests { assert!(is_valid_actor_name("a", actor_name_max_length).is_err()); // empty assert!(is_valid_actor_name("", actor_name_max_length).is_err()); + // newline + assert!(is_valid_actor_name( + r"Line1 + +Line3", + actor_name_max_length + ) + .is_err()); + assert!(is_valid_actor_name("Line1\nLine3", actor_name_max_length).is_err()); } #[test] From dce6c6bbf044a28de1718de571d1f3c89dbb8e50 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Tue, 12 Nov 2024 18:03:30 +0100 Subject: [PATCH 03/12] Move aggregates to replaceable_schema, fix error (fixes #5186) (#5190) * Move aggregates to replaceable_schema, fix error (fixes #5186) * fmt * r prefix --- crates/db_schema/replaceable_schema/utils.sql | 115 ++++++++++++++++++ .../2024-11-12-090437_move-triggers/down.sql | 115 ++++++++++++++++++ .../2024-11-12-090437_move-triggers/up.sql | 2 + src/scheduled_tasks.rs | 4 +- 4 files changed, 234 insertions(+), 2 deletions(-) create mode 100644 migrations/2024-11-12-090437_move-triggers/down.sql create mode 100644 migrations/2024-11-12-090437_move-triggers/up.sql diff --git a/crates/db_schema/replaceable_schema/utils.sql b/crates/db_schema/replaceable_schema/utils.sql index c766d25f2..0c7f42ff2 100644 --- a/crates/db_schema/replaceable_schema/utils.sql +++ b/crates/db_schema/replaceable_schema/utils.sql @@ -151,3 +151,118 @@ DECLARE END; $a$; +-- Edit community aggregates to include voters as active users +CREATE OR REPLACE FUNCTION r.community_aggregates_activity (i text) + RETURNS TABLE ( + count_ bigint, + community_id_ integer) + LANGUAGE plpgsql + AS $$ +BEGIN + RETURN query + SELECT + count(*), + community_id + FROM ( + SELECT + c.creator_id, + p.community_id + FROM + comment c + INNER JOIN post p ON c.post_id = p.id + INNER JOIN person pe ON c.creator_id = pe.id + WHERE + c.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE + UNION + SELECT + p.creator_id, + p.community_id + FROM + post p + INNER JOIN person pe ON p.creator_id = pe.id + WHERE + p.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE + UNION + SELECT + pl.person_id, + p.community_id + FROM + post_like pl + INNER JOIN post p ON pl.post_id = p.id + INNER JOIN person pe ON pl.person_id = pe.id + WHERE + pl.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE + UNION + SELECT + cl.person_id, + p.community_id + FROM + comment_like cl + INNER JOIN comment c ON cl.comment_id = c.id + INNER JOIN post p ON c.post_id = p.id + INNER JOIN person pe ON cl.person_id = pe.id + WHERE + cl.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE) a +GROUP BY + community_id; +END; +$$; + +-- Edit site aggregates to include voters and people who have read posts as active users +CREATE OR REPLACE FUNCTION r.site_aggregates_activity (i text) + RETURNS integer + LANGUAGE plpgsql + AS $$ +DECLARE + count_ integer; +BEGIN + SELECT + count(*) INTO count_ + FROM ( + SELECT + c.creator_id + FROM + comment c + INNER JOIN person pe ON c.creator_id = pe.id + WHERE + c.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE + UNION + SELECT + p.creator_id + FROM + post p + INNER JOIN person pe ON p.creator_id = pe.id + WHERE + p.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE + UNION + SELECT + pl.person_id + FROM + post_like pl + INNER JOIN person pe ON pl.person_id = pe.id + WHERE + pl.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE + UNION + SELECT + cl.person_id + FROM + comment_like cl + INNER JOIN person pe ON cl.person_id = pe.id + WHERE + cl.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE) a; + RETURN count_; +END; +$$; + diff --git a/migrations/2024-11-12-090437_move-triggers/down.sql b/migrations/2024-11-12-090437_move-triggers/down.sql new file mode 100644 index 000000000..3607679bc --- /dev/null +++ b/migrations/2024-11-12-090437_move-triggers/down.sql @@ -0,0 +1,115 @@ +-- Edit community aggregates to include voters as active users +CREATE OR REPLACE FUNCTION community_aggregates_activity (i text) + RETURNS TABLE ( + count_ bigint, + community_id_ integer) + LANGUAGE plpgsql + AS $$ +BEGIN + RETURN query + SELECT + count(*), + community_id + FROM ( + SELECT + c.creator_id, + p.community_id + FROM + comment c + INNER JOIN post p ON c.post_id = p.id + INNER JOIN person pe ON c.creator_id = pe.id + WHERE + c.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE + UNION + SELECT + p.creator_id, + p.community_id + FROM + post p + INNER JOIN person pe ON p.creator_id = pe.id + WHERE + p.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE + UNION + SELECT + pl.person_id, + p.community_id + FROM + post_like pl + INNER JOIN post p ON pl.post_id = p.id + INNER JOIN person pe ON pl.person_id = pe.id + WHERE + pl.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE + UNION + SELECT + cl.person_id, + p.community_id + FROM + comment_like cl + INNER JOIN comment c ON cl.comment_id = comment.id + INNER JOIN post p ON comment.post_id = p.id + INNER JOIN person pe ON cl.person_id = pe.id + WHERE + cl.published > ('now'::timestamp - i::interval) + AND pe.bot_account = FALSE) a +GROUP BY + community_id; +END; +$$; + +-- Edit site aggregates to include voters and people who have read posts as active users +CREATE OR REPLACE FUNCTION site_aggregates_activity (i text) + RETURNS integer + LANGUAGE plpgsql + AS $$ +DECLARE + count_ integer; +BEGIN + SELECT + count(*) INTO count_ + FROM ( + SELECT + c.creator_id + FROM + comment c + INNER JOIN person pe ON c.creator_id = pe.id + WHERE + c.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE + UNION + SELECT + p.creator_id + FROM + post p + INNER JOIN person pe ON p.creator_id = pe.id + WHERE + p.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE + UNION + SELECT + pl.person_id + FROM + post_like pl + INNER JOIN person pe ON pl.person_id = pe.id + WHERE + pl.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE + UNION + SELECT + cl.person_id + FROM + comment_like cl + INNER JOIN person pe ON cl.person_id = pe.id + WHERE + cl.published > ('now'::timestamp - i::interval) + AND pe.local = TRUE + AND pe.bot_account = FALSE) a; + RETURN count_; +END; +$$; + diff --git a/migrations/2024-11-12-090437_move-triggers/up.sql b/migrations/2024-11-12-090437_move-triggers/up.sql new file mode 100644 index 000000000..e7b2bd49d --- /dev/null +++ b/migrations/2024-11-12-090437_move-triggers/up.sql @@ -0,0 +1,2 @@ +DROP FUNCTION community_aggregates_activity, site_aggregates_activity CASCADE; + diff --git a/src/scheduled_tasks.rs b/src/scheduled_tasks.rs index 148df00fe..043d78d6b 100644 --- a/src/scheduled_tasks.rs +++ b/src/scheduled_tasks.rs @@ -403,7 +403,7 @@ async fn active_counts(pool: &mut DbPool<'_>) { for (full_form, abbr) in &intervals { let update_site_stmt = format!( - "update site_aggregates set users_active_{} = (select * from site_aggregates_activity('{}')) where site_id = 1", + "update site_aggregates set users_active_{} = (select * from r.site_aggregates_activity('{}')) where site_id = 1", abbr, full_form ); sql_query(update_site_stmt) @@ -412,7 +412,7 @@ async fn active_counts(pool: &mut DbPool<'_>) { .inspect_err(|e| error!("Failed to update site stats: {e}")) .ok(); - let update_community_stmt = format!("update community_aggregates ca set users_active_{} = mv.count_ from community_aggregates_activity('{}') mv where ca.community_id = mv.community_id_", abbr, full_form); + let update_community_stmt = format!("update community_aggregates ca set users_active_{} = mv.count_ from r.community_aggregates_activity('{}') mv where ca.community_id = mv.community_id_", abbr, full_form); sql_query(update_community_stmt) .execute(&mut conn) .await From 542e59bcaed256c88a9d974e6c6c3dc59f7ebbc9 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Tue, 12 Nov 2024 18:43:24 +0100 Subject: [PATCH 04/12] Fetch community mods synchronously (#5169) * Fetch community mods synchronously * fix * fix --------- Co-authored-by: Dessalines --- crates/apub/src/objects/community.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs index efa2c5247..2a9abc939 100644 --- a/crates/apub/src/objects/community.rs +++ b/crates/apub/src/objects/community.rs @@ -194,10 +194,16 @@ impl Object for ApubCommunity { LanguageTag::to_language_id_multiple(group.language, &mut context.pool()).await?; let timestamp = group.updated.or(group.published).unwrap_or_else(naive_now); - let community = Community::insert_apub(&mut context.pool(), timestamp, &form).await?; + let community: ApubCommunity = Community::insert_apub(&mut context.pool(), timestamp, &form) + .await? + .into(); CommunityLanguage::update(&mut context.pool(), languages, community.id).await?; - let community: ApubCommunity = community.into(); + // Need to fetch mods synchronously, otherwise fetching a post in community with + // `posting_restricted_to_mods` can fail if mods havent been fetched yet. + if let Some(moderators) = group.attributed_to { + moderators.dereference(&community, context).await.ok(); + } // These collections are not necessary for Lemmy to work, so ignore errors. let community_ = community.clone(); @@ -210,9 +216,6 @@ impl Object for ApubCommunity { if let Some(featured) = group.featured { featured.dereference(&community_, &context_).await.ok(); } - if let Some(moderators) = group.attributed_to { - moderators.dereference(&community_, &context_).await.ok(); - } Ok(()) }); From f916309df827c19d0df1cc930dde508515687a5c Mon Sep 17 00:00:00 2001 From: Nutomic Date: Tue, 12 Nov 2024 20:52:39 +0100 Subject: [PATCH 05/12] Always assign default language before checking if language is allowed (#5132) * Always assign default language before checking if language is allowed (fixes #5131) * handle all logic in same fn * fix test * rename --- crates/api_crud/src/comment/create.rs | 25 +++------ crates/api_crud/src/comment/update.rs | 19 ++++--- crates/api_crud/src/post/create.rs | 27 +++------- crates/api_crud/src/post/update.rs | 19 ++++--- crates/db_schema/src/impls/actor_language.rs | 56 ++++++++++++-------- 5 files changed, 68 insertions(+), 78 deletions(-) diff --git a/crates/api_crud/src/comment/create.rs b/crates/api_crud/src/comment/create.rs index 65aa1f612..edcf7db30 100644 --- a/crates/api_crud/src/comment/create.rs +++ b/crates/api_crud/src/comment/create.rs @@ -16,9 +16,8 @@ use lemmy_api_common::{ }, }; use lemmy_db_schema::{ - impls::actor_language::default_post_language, + impls::actor_language::validate_post_language, source::{ - actor_language::CommunityLanguage, comment::{Comment, CommentInsertForm, CommentLike, CommentLikeForm}, comment_reply::{CommentReply, CommentReplyUpdateForm}, local_site::LocalSite, @@ -93,21 +92,13 @@ pub async fn create_comment( check_comment_depth(parent)?; } - // attempt to set default language if none was provided - let language_id = match data.language_id { - Some(lid) => lid, - None => { - default_post_language( - &mut context.pool(), - community_id, - local_user_view.local_user.id, - ) - .await? - } - }; - - CommunityLanguage::is_allowed_community_language(&mut context.pool(), language_id, community_id) - .await?; + let language_id = validate_post_language( + &mut context.pool(), + data.language_id, + community_id, + local_user_view.local_user.id, + ) + .await?; let comment_form = CommentInsertForm { language_id: Some(language_id), diff --git a/crates/api_crud/src/comment/update.rs b/crates/api_crud/src/comment/update.rs index 95cc85fe4..85ee9bff0 100644 --- a/crates/api_crud/src/comment/update.rs +++ b/crates/api_crud/src/comment/update.rs @@ -13,8 +13,8 @@ use lemmy_api_common::{ }, }; use lemmy_db_schema::{ + impls::actor_language::validate_post_language, source::{ - actor_language::CommunityLanguage, comment::{Comment, CommentUpdateForm}, local_site::LocalSite, }, @@ -55,14 +55,13 @@ pub async fn update_comment( Err(LemmyErrorType::NoCommentEditAllowed)? } - if let Some(language_id) = data.language_id { - CommunityLanguage::is_allowed_community_language( - &mut context.pool(), - language_id, - orig_comment.community.id, - ) - .await?; - } + let language_id = validate_post_language( + &mut context.pool(), + data.language_id, + orig_comment.community.id, + local_user_view.local_user.id, + ) + .await?; let slur_regex = local_site_to_slur_regex(&local_site); let url_blocklist = get_url_blocklist(&context).await?; @@ -74,7 +73,7 @@ pub async fn update_comment( let comment_id = data.comment_id; let form = CommentUpdateForm { content, - language_id: data.language_id, + language_id: Some(language_id), updated: Some(Some(naive_now())), ..Default::default() }; diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index 16932cacb..5dee5e21c 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -17,9 +17,8 @@ use lemmy_api_common::{ }, }; use lemmy_db_schema::{ - impls::actor_language::default_post_language, + impls::actor_language::validate_post_language, source::{ - actor_language::CommunityLanguage, community::Community, local_site::LocalSite, post::{Post, PostInsertForm, PostLike, PostLikeForm}, @@ -98,23 +97,13 @@ pub async fn create_post( .await?; } - // attempt to set default language if none was provided - let language_id = match data.language_id { - Some(lid) => lid, - None => { - default_post_language( - &mut context.pool(), - community.id, - local_user_view.local_user.id, - ) - .await? - } - }; - - // Only need to check if language is allowed in case user set it explicitly. When using default - // language, it already only returns allowed languages. - CommunityLanguage::is_allowed_community_language(&mut context.pool(), language_id, community.id) - .await?; + let language_id = validate_post_language( + &mut context.pool(), + data.language_id, + data.community_id, + local_user_view.local_user.id, + ) + .await?; let scheduled_publish_time = convert_published_time(data.scheduled_publish_time, &local_user_view, &context).await?; diff --git a/crates/api_crud/src/post/update.rs b/crates/api_crud/src/post/update.rs index fc23e7d9e..217a6cc24 100644 --- a/crates/api_crud/src/post/update.rs +++ b/crates/api_crud/src/post/update.rs @@ -15,8 +15,8 @@ use lemmy_api_common::{ }, }; use lemmy_db_schema::{ + impls::actor_language::validate_post_language, source::{ - actor_language::CommunityLanguage, community::Community, local_site::LocalSite, post::{Post, PostUpdateForm}, @@ -101,14 +101,13 @@ pub async fn update_post( Err(LemmyErrorType::NoPostEditAllowed)? } - if let Some(language_id) = data.language_id { - CommunityLanguage::is_allowed_community_language( - &mut context.pool(), - language_id, - orig_post.community.id, - ) - .await?; - } + let language_id = validate_post_language( + &mut context.pool(), + data.language_id, + orig_post.post.community_id, + local_user_view.local_user.id, + ) + .await?; // handle changes to scheduled_publish_time let scheduled_publish_time = match ( @@ -131,7 +130,7 @@ pub async fn update_post( body, alt_text, nsfw: data.nsfw, - language_id: data.language_id, + language_id: Some(language_id), updated: Some(Some(naive_now())), scheduled_publish_time, ..Default::default() diff --git a/crates/db_schema/src/impls/actor_language.rs b/crates/db_schema/src/impls/actor_language.rs index b4ad0d347..8d4c471be 100644 --- a/crates/db_schema/src/impls/actor_language.rs +++ b/crates/db_schema/src/impls/actor_language.rs @@ -197,7 +197,7 @@ impl SiteLanguage { impl CommunityLanguage { /// Returns true if the given language is one of configured languages for given community - pub async fn is_allowed_community_language( + async fn is_allowed_community_language( pool: &mut DbPool<'_>, for_language_id: LanguageId, for_community_id: CommunityId, @@ -319,29 +319,38 @@ impl CommunityLanguage { } } -pub async fn default_post_language( +pub async fn validate_post_language( pool: &mut DbPool<'_>, + language_id: Option, community_id: CommunityId, local_user_id: LocalUserId, -) -> Result { +) -> LemmyResult { use crate::schema::{community_language::dsl as cl, local_user_language::dsl as ul}; let conn = &mut get_conn(pool).await?; - let mut intersection = ul::local_user_language - .inner_join(cl::community_language.on(ul::language_id.eq(cl::language_id))) - .filter(ul::local_user_id.eq(local_user_id)) - .filter(cl::community_id.eq(community_id)) - .select(cl::language_id) - .get_results::(conn) - .await?; + let language_id = match language_id { + None | Some(LanguageId(0)) => { + let mut intersection = ul::local_user_language + .inner_join(cl::community_language.on(ul::language_id.eq(cl::language_id))) + .filter(ul::local_user_id.eq(local_user_id)) + .filter(cl::community_id.eq(community_id)) + .select(cl::language_id) + .get_results::(conn) + .await?; - if intersection.len() == 1 { - Ok(intersection.pop().unwrap_or(UNDETERMINED_ID)) - } else if intersection.len() == 2 && intersection.contains(&UNDETERMINED_ID) { - intersection.retain(|i| i != &UNDETERMINED_ID); - Ok(intersection.pop().unwrap_or(UNDETERMINED_ID)) - } else { - Ok(UNDETERMINED_ID) - } + if intersection.len() == 1 { + intersection.pop().unwrap_or(UNDETERMINED_ID) + } else if intersection.len() == 2 && intersection.contains(&UNDETERMINED_ID) { + intersection.retain(|i| i != &UNDETERMINED_ID); + intersection.pop().unwrap_or(UNDETERMINED_ID) + } else { + UNDETERMINED_ID + } + } + Some(lid) => lid, + }; + + CommunityLanguage::is_allowed_community_language(pool, language_id, community_id).await?; + Ok(language_id) } /// If no language is given, set all languages @@ -590,7 +599,7 @@ mod tests { #[tokio::test] #[serial] - async fn test_default_post_language() -> Result<(), Error> { + async fn test_validate_post_language() -> LemmyResult<()> { let pool = &build_db_pool_for_tests(); let pool = &mut pool.into(); let (site, instance) = create_test_site(pool).await?; @@ -613,8 +622,11 @@ mod tests { LocalUserLanguage::update(pool, test_langs2, local_user.id).await?; // no overlap in user/community languages, so defaults to undetermined - let def1 = default_post_language(pool, community.id, local_user.id).await?; - assert_eq!(UNDETERMINED_ID, def1); + let def1 = validate_post_language(pool, None, community.id, local_user.id).await; + assert_eq!( + Some(LemmyErrorType::LanguageNotAllowed), + def1.err().map(|e| e.error_type) + ); let ru = Language::read_id_from_code(pool, "ru").await?; let test_langs3 = vec![ @@ -626,7 +638,7 @@ mod tests { LocalUserLanguage::update(pool, test_langs3, local_user.id).await?; // this time, both have ru as common lang - let def2 = default_post_language(pool, community.id, local_user.id).await?; + let def2 = validate_post_language(pool, None, community.id, local_user.id).await?; assert_eq!(ru, def2); Person::delete(pool, person.id).await?; From faf62de4e32c7cc21e76a2cfa92ccf523f4f39ae Mon Sep 17 00:00:00 2001 From: Dessalines Date: Wed, 13 Nov 2024 03:45:17 -0500 Subject: [PATCH 06/12] Fixing cors origin wildcard. (#5194) * Fixing cors origin wildcard. - Fixes #5185 * Add other allows to specified origin block. * Fix clippy. --- config/defaults.hjson | 2 +- crates/utils/src/settings/structs.rs | 2 +- src/lib.rs | 24 ++++++++++++++++-------- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/config/defaults.hjson b/config/defaults.hjson index 96dc30b79..c12f879c7 100644 --- a/config/defaults.hjson +++ b/config/defaults.hjson @@ -122,5 +122,5 @@ } # Sets a response Access-Control-Allow-Origin CORS header # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin - cors_origin: "*" + cors_origin: "lemmy.tld" } diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index c95f66644..e8106d482 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -52,7 +52,7 @@ pub struct Settings { /// Sets a response Access-Control-Allow-Origin CORS header /// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin #[default(None)] - #[doku(example = "*")] + #[doku(example = "lemmy.tld")] cors_origin: Option, } diff --git a/src/lib.rs b/src/lib.rs index 9da09f65b..b94b5eab1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -339,23 +339,31 @@ fn create_http_server( fn cors_config(settings: &Settings) -> Cors { let self_origin = settings.get_protocol_and_hostname(); let cors_origin_setting = settings.cors_origin(); + + // A default setting for either wildcard, or None + let cors_default = Cors::default() + .allow_any_origin() + .allow_any_method() + .allow_any_header() + .expose_any_header() + .max_age(3600); + match (cors_origin_setting.clone(), cfg!(debug_assertions)) { (Some(origin), false) => { // Need to call send_wildcard() explicitly, passing this into allowed_origin() results in // error - if cors_origin_setting.as_deref() == Some("*") { - Cors::default().allow_any_origin().send_wildcard() + if origin == "*" { + cors_default } else { Cors::default() .allowed_origin(&origin) .allowed_origin(&self_origin) + .allow_any_method() + .allow_any_header() + .expose_any_header() + .max_age(3600) } } - _ => Cors::default() - .allow_any_origin() - .allow_any_method() - .allow_any_header() - .expose_any_header() - .max_age(3600), + _ => cors_default, } } From c4d864878f9f4b4005fadf60bcdafb345bd36988 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Wed, 13 Nov 2024 09:36:18 -0500 Subject: [PATCH 07/12] Change "mark post as read", "hide post" api calls to take single post instead of multiple (#5043) * Removing a few SuccessResponses for PostHide and MarkPostAsRead. - This also removes the pointless multiple post_ids. These can be done as individual calls on the front end anyway. - Fixes #4755 * Fixing federation tests. * Upgrading lemmy-js-client deps. * Simplifying forms. * Fixing forms. * Removing indexing slicing from a test. --- api_tests/package.json | 12 +- api_tests/pnpm-lock.yaml | 248 +++++++++++++++-------------- api_tests/src/shared.ts | 5 +- crates/api/src/post/hide.rs | 33 ++-- crates/api/src/post/mark_read.rs | 32 ++-- crates/api_common/src/post.rs | 4 +- crates/api_common/src/utils.rs | 4 +- crates/db_schema/src/impls/post.rs | 76 ++++----- crates/db_views/src/post_view.rs | 9 +- 9 files changed, 215 insertions(+), 208 deletions(-) diff --git a/api_tests/package.json b/api_tests/package.json index 9a5057c00..57173595a 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -22,16 +22,16 @@ }, "devDependencies": { "@types/jest": "^29.5.12", - "@types/node": "^22.3.0", - "@typescript-eslint/eslint-plugin": "^8.1.0", - "@typescript-eslint/parser": "^8.1.0", - "eslint": "^9.9.0", + "@types/node": "^22.9.0", + "@typescript-eslint/eslint-plugin": "^8.13.0", + "@typescript-eslint/parser": "^8.13.0", + "eslint": "^9.14.0", "eslint-plugin-prettier": "^5.1.3", "jest": "^29.5.0", - "lemmy-js-client": "0.20.0-private-community.9", + "lemmy-js-client": "0.20.0-alpha.18", "prettier": "^3.2.5", "ts-jest": "^29.1.0", "typescript": "^5.5.4", - "typescript-eslint": "^8.1.0" + "typescript-eslint": "^8.13.0" } } diff --git a/api_tests/pnpm-lock.yaml b/api_tests/pnpm-lock.yaml index b1f18622e..01d4a8e74 100644 --- a/api_tests/pnpm-lock.yaml +++ b/api_tests/pnpm-lock.yaml @@ -12,38 +12,38 @@ importers: specifier: ^29.5.12 version: 29.5.14 '@types/node': - specifier: ^22.3.0 - version: 22.8.6 + specifier: ^22.9.0 + version: 22.9.0 '@typescript-eslint/eslint-plugin': - specifier: ^8.1.0 - version: 8.12.2(@typescript-eslint/parser@8.12.2(eslint@9.13.0)(typescript@5.6.3))(eslint@9.13.0)(typescript@5.6.3) + specifier: ^8.13.0 + version: 8.13.0(@typescript-eslint/parser@8.13.0(eslint@9.14.0)(typescript@5.6.3))(eslint@9.14.0)(typescript@5.6.3) '@typescript-eslint/parser': - specifier: ^8.1.0 - version: 8.12.2(eslint@9.13.0)(typescript@5.6.3) + specifier: ^8.13.0 + version: 8.13.0(eslint@9.14.0)(typescript@5.6.3) eslint: - specifier: ^9.9.0 - version: 9.13.0 + specifier: ^9.14.0 + version: 9.14.0 eslint-plugin-prettier: specifier: ^5.1.3 - version: 5.2.1(eslint@9.13.0)(prettier@3.3.3) + version: 5.2.1(eslint@9.14.0)(prettier@3.3.3) jest: specifier: ^29.5.0 - version: 29.7.0(@types/node@22.8.6) + version: 29.7.0(@types/node@22.9.0) lemmy-js-client: - specifier: 0.20.0-private-community.9 - version: 0.20.0-private-community.9 + specifier: 0.20.0-alpha.18 + version: 0.20.0-alpha.18 prettier: specifier: ^3.2.5 version: 3.3.3 ts-jest: specifier: ^29.1.0 - version: 29.2.5(@babel/core@7.23.9)(@jest/transform@29.7.0)(@jest/types@29.6.3)(babel-jest@29.7.0(@babel/core@7.23.9))(jest@29.7.0(@types/node@22.8.6))(typescript@5.6.3) + version: 29.2.5(@babel/core@7.23.9)(@jest/transform@29.7.0)(@jest/types@29.6.3)(babel-jest@29.7.0(@babel/core@7.23.9))(jest@29.7.0(@types/node@22.9.0))(typescript@5.6.3) typescript: specifier: ^5.5.4 version: 5.6.3 typescript-eslint: - specifier: ^8.1.0 - version: 8.12.2(eslint@9.13.0)(typescript@5.6.3) + specifier: ^8.13.0 + version: 8.13.0(eslint@9.14.0)(typescript@5.6.3) packages: @@ -240,8 +240,8 @@ packages: resolution: {integrity: sha512-4Bfj15dVJdoy3RfZmmo86RK1Fwzn6SstsvK9JS+BaVKqC6QQQQyXekNaC+g+LKNgkQ+2VhGAzm6hO40AhMR3zQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} - '@eslint/js@9.13.0': - resolution: {integrity: sha512-IFLyoY4d72Z5y/6o/BazFBezupzI/taV8sGumxTAVw3lXG9A6md1Dc34T9s1FoD/an9pJH8RHbAxsaEbBed9lA==} + '@eslint/js@9.14.0': + resolution: {integrity: sha512-pFoEtFWCPyDOl+C6Ift+wC7Ro89otjigCf5vcuWqWgqNSQbRrpjSvdeE6ofLz4dHmyxD5f7gIdGT4+p36L6Twg==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} '@eslint/object-schema@2.1.4': @@ -268,6 +268,10 @@ packages: resolution: {integrity: sha512-JBxkERygn7Bv/GbN5Rv8Ul6LVknS+5Bp6RgDC/O8gEBU/yeH5Ui5C/OlWrTb6qct7LjjfT6Re2NxB0ln0yYybA==} engines: {node: '>=18.18'} + '@humanwhocodes/retry@0.4.1': + resolution: {integrity: sha512-c7hNEllBlenFTHBky65mhq8WD2kbN9Q6gk0bTk8lSBvc554jpXSkST1iePudpt7+A/AQvuHs9EMqjHDXMY1lrA==} + engines: {node: '>=18.18'} + '@istanbuljs/load-nyc-config@1.1.0': resolution: {integrity: sha512-VjeHSlIzpv/NyD3N0YuHfXOPDIixcA1q2ZV98wsMqcYlPmv2n3Yb2lYP9XMElnaFVXg5A7YLTeLu6V84uQDjmQ==} engines: {node: '>=8'} @@ -418,8 +422,8 @@ packages: '@types/json-schema@7.0.15': resolution: {integrity: sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA==} - '@types/node@22.8.6': - resolution: {integrity: sha512-tosuJYKrIqjQIlVCM4PEGxOmyg3FCPa/fViuJChnGeEIhjA46oy8FMVoF9su1/v8PNs2a8Q0iFNyOx0uOF91nw==} + '@types/node@22.9.0': + resolution: {integrity: sha512-vuyHg81vvWA1Z1ELfvLko2c8f34gyA0zaic0+Rllc5lbCnbSyuvb2Oxpm6TAUAC/2xZN3QGqxBNggD1nNR2AfQ==} '@types/stack-utils@2.0.3': resolution: {integrity: sha512-9aEbYZ3TbYMznPdcdr3SmIrLXwC/AKZXQeCf9Pgao5CKb8CyHuEX5jzWPTkvregvhRJHcpRO6BFoGW9ycaOkYw==} @@ -430,8 +434,8 @@ packages: '@types/yargs@17.0.32': resolution: {integrity: sha512-xQ67Yc/laOG5uMfX/093MRlGGCIBzZMarVa+gfNKJxWAIgykYpVGkBdbqEzGDDfCrVUj6Hiff4mTZ5BA6TmAog==} - '@typescript-eslint/eslint-plugin@8.12.2': - resolution: {integrity: sha512-gQxbxM8mcxBwaEmWdtLCIGLfixBMHhQjBqR8sVWNTPpcj45WlYL2IObS/DNMLH1DBP0n8qz+aiiLTGfopPEebw==} + '@typescript-eslint/eslint-plugin@8.13.0': + resolution: {integrity: sha512-nQtBLiZYMUPkclSeC3id+x4uVd1SGtHuElTxL++SfP47jR0zfkZBJHc+gL4qPsgTuypz0k8Y2GheaDYn6Gy3rg==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: '@typescript-eslint/parser': ^8.0.0 || ^8.0.0-alpha.0 @@ -441,8 +445,8 @@ packages: typescript: optional: true - '@typescript-eslint/parser@8.12.2': - resolution: {integrity: sha512-MrvlXNfGPLH3Z+r7Tk+Z5moZAc0dzdVjTgUgwsdGweH7lydysQsnSww3nAmsq8blFuRD5VRlAr9YdEFw3e6PBw==} + '@typescript-eslint/parser@8.13.0': + resolution: {integrity: sha512-w0xp+xGg8u/nONcGw1UXAr6cjCPU1w0XVyBs6Zqaj5eLmxkKQAByTdV/uGgNN5tVvN/kKpoQlP2cL7R+ajZZIQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: eslint: ^8.57.0 || ^9.0.0 @@ -451,12 +455,12 @@ packages: typescript: optional: true - '@typescript-eslint/scope-manager@8.12.2': - resolution: {integrity: sha512-gPLpLtrj9aMHOvxJkSbDBmbRuYdtiEbnvO25bCMza3DhMjTQw0u7Y1M+YR5JPbMsXXnSPuCf5hfq0nEkQDL/JQ==} + '@typescript-eslint/scope-manager@8.13.0': + resolution: {integrity: sha512-XsGWww0odcUT0gJoBZ1DeulY1+jkaHUciUq4jKNv4cpInbvvrtDoyBH9rE/n2V29wQJPk8iCH1wipra9BhmiMA==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} - '@typescript-eslint/type-utils@8.12.2': - resolution: {integrity: sha512-bwuU4TAogPI+1q/IJSKuD4shBLc/d2vGcRT588q+jzayQyjVK2X6v/fbR4InY2U2sgf8MEvVCqEWUzYzgBNcGQ==} + '@typescript-eslint/type-utils@8.13.0': + resolution: {integrity: sha512-Rqnn6xXTR316fP4D2pohZenJnp+NwQ1mo7/JM+J1LWZENSLkJI8ID8QNtlvFeb0HnFSK94D6q0cnMX6SbE5/vA==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: typescript: '*' @@ -464,12 +468,12 @@ packages: typescript: optional: true - '@typescript-eslint/types@8.12.2': - resolution: {integrity: sha512-VwDwMF1SZ7wPBUZwmMdnDJ6sIFk4K4s+ALKLP6aIQsISkPv8jhiw65sAK6SuWODN/ix+m+HgbYDkH+zLjrzvOA==} + '@typescript-eslint/types@8.13.0': + resolution: {integrity: sha512-4cyFErJetFLckcThRUFdReWJjVsPCqyBlJTi6IDEpc1GWCIIZRFxVppjWLIMcQhNGhdWJJRYFHpHoDWvMlDzng==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} - '@typescript-eslint/typescript-estree@8.12.2': - resolution: {integrity: sha512-mME5MDwGe30Pq9zKPvyduyU86PH7aixwqYR2grTglAdB+AN8xXQ1vFGpYaUSJ5o5P/5znsSBeNcs5g5/2aQwow==} + '@typescript-eslint/typescript-estree@8.13.0': + resolution: {integrity: sha512-v7SCIGmVsRK2Cy/LTLGN22uea6SaUIlpBcO/gnMGT/7zPtxp90bphcGf4fyrCQl3ZtiBKqVTG32hb668oIYy1g==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: typescript: '*' @@ -477,14 +481,14 @@ packages: typescript: optional: true - '@typescript-eslint/utils@8.12.2': - resolution: {integrity: sha512-UTTuDIX3fkfAz6iSVa5rTuSfWIYZ6ATtEocQ/umkRSyC9O919lbZ8dcH7mysshrCdrAM03skJOEYaBugxN+M6A==} + '@typescript-eslint/utils@8.13.0': + resolution: {integrity: sha512-A1EeYOND6Uv250nybnLZapeXpYMl8tkzYUxqmoKAWnI4sei3ihf2XdZVd+vVOmHGcp3t+P7yRrNsyyiXTvShFQ==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: eslint: ^8.57.0 || ^9.0.0 - '@typescript-eslint/visitor-keys@8.12.2': - resolution: {integrity: sha512-PChz8UaKQAVNHghsHcPyx1OMHoFRUEA7rJSK/mDhdq85bk+PLsUHUBqTQTFt18VJZbmxBovM65fezlheQRsSDA==} + '@typescript-eslint/visitor-keys@8.13.0': + resolution: {integrity: sha512-7N/+lztJqH4Mrf0lb10R/CbI1EaAMMGyF5y0oJvFoAhafwgiRA7TXyd8TFn8FC8k5y2dTsYogg238qavRGNnlw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} acorn-jsx@5.3.2: @@ -649,6 +653,10 @@ packages: resolution: {integrity: sha512-iRDPJKUPVEND7dHPO8rkbOnPpyDygcDFtWjpeWNCgy8WP2rXcxXL8TskReQl6OrB2G7+UJrags1q15Fudc7G6w==} engines: {node: '>= 8'} + cross-spawn@7.0.5: + resolution: {integrity: sha512-ZVJrKKYunU38/76t0RMOulHOnUcbU9GbpWKAOZ0mhjr7CX6FVrH+4FrAapSOekrgFQ3f/8gwMEuIft0aKq6Hug==} + engines: {node: '>= 8'} + debug@4.3.7: resolution: {integrity: sha512-Er2nc/H7RrMXZBFCEim6TCmMk02Z8vLC2Rbi1KEBggpo0fS6l0S1nnapwmIi3yW/+GOJap1Krg4w0Hg80oCqgQ==} engines: {node: '>=6.0'} @@ -737,8 +745,8 @@ packages: resolution: {integrity: sha512-UyLnSehNt62FFhSwjZlHmeokpRK59rcz29j+F1/aDgbkbRTk7wIc9XzdoasMUbRNKDM0qQt/+BJ4BrpFeABemw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} - eslint@9.13.0: - resolution: {integrity: sha512-EYZK6SX6zjFHST/HRytOdA/zE72Cq/bfw45LSyuwrdvcclb/gqV8RRQxywOBEWO2+WDpva6UZa4CcDeJKzUCFA==} + eslint@9.14.0: + resolution: {integrity: sha512-c2FHsVBr87lnUtjP4Yhvk4yEhKrQavGafRA/Se1ouse8PfbfC/Qh9Mxa00yWsZRlqeUB9raXip0aiiUZkgnr9g==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} hasBin: true peerDependencies: @@ -1159,8 +1167,8 @@ packages: resolution: {integrity: sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w==} engines: {node: '>=6'} - lemmy-js-client@0.20.0-private-community.9: - resolution: {integrity: sha512-iuFezswCzIco5U5Q4Eo8HAWVE65pDW2zeO+fYLEyFl30SLw9a3gqJkip2vbDfVvoAjDXyUskZKddf1Nnj8mVcg==} + lemmy-js-client@0.20.0-alpha.18: + resolution: {integrity: sha512-oZy8DboTWfUar4mPWpi7SYrOEjTBJxkvd1e6QaVwoA5UhqQV1WhxEYbzrpi/gXnEokaVQ0i5sjtL/Y2PHMO3MQ==} leven@3.1.0: resolution: {integrity: sha512-qsda+H8jTaUaN/x5vzW2rzc+8Rw4TAQ/4KjB46IwK5VH+IlVeeeje/EoZRpiXvIqjFgK84QffqPztGI3VBLG1A==} @@ -1533,8 +1541,8 @@ packages: resolution: {integrity: sha512-t0rzBq87m3fVcduHDUFhKmyyX+9eo6WQjZvf51Ea/M0Q7+T374Jp1aUiyUl0GKxp8M/OETVHSDvmkyPgvX+X2w==} engines: {node: '>=10'} - typescript-eslint@8.12.2: - resolution: {integrity: sha512-UbuVUWSrHVR03q9CWx+JDHeO6B/Hr9p4U5lRH++5tq/EbFq1faYZe50ZSBePptgfIKLEti0aPQ3hFgnPVcd8ZQ==} + typescript-eslint@8.13.0: + resolution: {integrity: sha512-vIMpDRJrQd70au2G8w34mPps0ezFSPMEX4pXkTzUkrNbRX+36ais2ksGWN0esZL+ZMaFJEneOBHzCgSqle7DHw==} engines: {node: ^18.18.0 || ^20.9.0 || >=21.1.0} peerDependencies: typescript: '*' @@ -1808,9 +1816,9 @@ snapshots: '@bcoe/v8-coverage@0.2.3': {} - '@eslint-community/eslint-utils@4.4.1(eslint@9.13.0)': + '@eslint-community/eslint-utils@4.4.1(eslint@9.14.0)': dependencies: - eslint: 9.13.0 + eslint: 9.14.0 eslint-visitor-keys: 3.4.3 '@eslint-community/regexpp@4.12.1': {} @@ -1839,7 +1847,7 @@ snapshots: transitivePeerDependencies: - supports-color - '@eslint/js@9.13.0': {} + '@eslint/js@9.14.0': {} '@eslint/object-schema@2.1.4': {} @@ -1858,6 +1866,8 @@ snapshots: '@humanwhocodes/retry@0.3.1': {} + '@humanwhocodes/retry@0.4.1': {} + '@istanbuljs/load-nyc-config@1.1.0': dependencies: camelcase: 5.3.1 @@ -1871,7 +1881,7 @@ snapshots: '@jest/console@29.7.0': dependencies: '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 chalk: 4.1.2 jest-message-util: 29.7.0 jest-util: 29.7.0 @@ -1884,14 +1894,14 @@ snapshots: '@jest/test-result': 29.7.0 '@jest/transform': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 ansi-escapes: 4.3.2 chalk: 4.1.2 ci-info: 3.9.0 exit: 0.1.2 graceful-fs: 4.2.11 jest-changed-files: 29.7.0 - jest-config: 29.7.0(@types/node@22.8.6) + jest-config: 29.7.0(@types/node@22.9.0) jest-haste-map: 29.7.0 jest-message-util: 29.7.0 jest-regex-util: 29.6.3 @@ -1916,7 +1926,7 @@ snapshots: dependencies: '@jest/fake-timers': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 jest-mock: 29.7.0 '@jest/expect-utils@29.7.0': @@ -1934,7 +1944,7 @@ snapshots: dependencies: '@jest/types': 29.6.3 '@sinonjs/fake-timers': 10.3.0 - '@types/node': 22.8.6 + '@types/node': 22.9.0 jest-message-util: 29.7.0 jest-mock: 29.7.0 jest-util: 29.7.0 @@ -1956,7 +1966,7 @@ snapshots: '@jest/transform': 29.7.0 '@jest/types': 29.6.3 '@jridgewell/trace-mapping': 0.3.22 - '@types/node': 22.8.6 + '@types/node': 22.9.0 chalk: 4.1.2 collect-v8-coverage: 1.0.2 exit: 0.1.2 @@ -2026,7 +2036,7 @@ snapshots: '@jest/schemas': 29.6.3 '@types/istanbul-lib-coverage': 2.0.6 '@types/istanbul-reports': 3.0.4 - '@types/node': 22.8.6 + '@types/node': 22.9.0 '@types/yargs': 17.0.32 chalk: 4.1.2 @@ -2096,7 +2106,7 @@ snapshots: '@types/graceful-fs@4.1.9': dependencies: - '@types/node': 22.8.6 + '@types/node': 22.9.0 '@types/istanbul-lib-coverage@2.0.6': {} @@ -2115,7 +2125,7 @@ snapshots: '@types/json-schema@7.0.15': {} - '@types/node@22.8.6': + '@types/node@22.9.0': dependencies: undici-types: 6.19.8 @@ -2127,15 +2137,15 @@ snapshots: dependencies: '@types/yargs-parser': 21.0.3 - '@typescript-eslint/eslint-plugin@8.12.2(@typescript-eslint/parser@8.12.2(eslint@9.13.0)(typescript@5.6.3))(eslint@9.13.0)(typescript@5.6.3)': + '@typescript-eslint/eslint-plugin@8.13.0(@typescript-eslint/parser@8.13.0(eslint@9.14.0)(typescript@5.6.3))(eslint@9.14.0)(typescript@5.6.3)': dependencies: '@eslint-community/regexpp': 4.12.1 - '@typescript-eslint/parser': 8.12.2(eslint@9.13.0)(typescript@5.6.3) - '@typescript-eslint/scope-manager': 8.12.2 - '@typescript-eslint/type-utils': 8.12.2(eslint@9.13.0)(typescript@5.6.3) - '@typescript-eslint/utils': 8.12.2(eslint@9.13.0)(typescript@5.6.3) - '@typescript-eslint/visitor-keys': 8.12.2 - eslint: 9.13.0 + '@typescript-eslint/parser': 8.13.0(eslint@9.14.0)(typescript@5.6.3) + '@typescript-eslint/scope-manager': 8.13.0 + '@typescript-eslint/type-utils': 8.13.0(eslint@9.14.0)(typescript@5.6.3) + '@typescript-eslint/utils': 8.13.0(eslint@9.14.0)(typescript@5.6.3) + '@typescript-eslint/visitor-keys': 8.13.0 + eslint: 9.14.0 graphemer: 1.4.0 ignore: 5.3.2 natural-compare: 1.4.0 @@ -2145,28 +2155,28 @@ snapshots: transitivePeerDependencies: - supports-color - '@typescript-eslint/parser@8.12.2(eslint@9.13.0)(typescript@5.6.3)': + '@typescript-eslint/parser@8.13.0(eslint@9.14.0)(typescript@5.6.3)': dependencies: - '@typescript-eslint/scope-manager': 8.12.2 - '@typescript-eslint/types': 8.12.2 - '@typescript-eslint/typescript-estree': 8.12.2(typescript@5.6.3) - '@typescript-eslint/visitor-keys': 8.12.2 + '@typescript-eslint/scope-manager': 8.13.0 + '@typescript-eslint/types': 8.13.0 + '@typescript-eslint/typescript-estree': 8.13.0(typescript@5.6.3) + '@typescript-eslint/visitor-keys': 8.13.0 debug: 4.3.7 - eslint: 9.13.0 + eslint: 9.14.0 optionalDependencies: typescript: 5.6.3 transitivePeerDependencies: - supports-color - '@typescript-eslint/scope-manager@8.12.2': + '@typescript-eslint/scope-manager@8.13.0': dependencies: - '@typescript-eslint/types': 8.12.2 - '@typescript-eslint/visitor-keys': 8.12.2 + '@typescript-eslint/types': 8.13.0 + '@typescript-eslint/visitor-keys': 8.13.0 - '@typescript-eslint/type-utils@8.12.2(eslint@9.13.0)(typescript@5.6.3)': + '@typescript-eslint/type-utils@8.13.0(eslint@9.14.0)(typescript@5.6.3)': dependencies: - '@typescript-eslint/typescript-estree': 8.12.2(typescript@5.6.3) - '@typescript-eslint/utils': 8.12.2(eslint@9.13.0)(typescript@5.6.3) + '@typescript-eslint/typescript-estree': 8.13.0(typescript@5.6.3) + '@typescript-eslint/utils': 8.13.0(eslint@9.14.0)(typescript@5.6.3) debug: 4.3.7 ts-api-utils: 1.4.0(typescript@5.6.3) optionalDependencies: @@ -2175,12 +2185,12 @@ snapshots: - eslint - supports-color - '@typescript-eslint/types@8.12.2': {} + '@typescript-eslint/types@8.13.0': {} - '@typescript-eslint/typescript-estree@8.12.2(typescript@5.6.3)': + '@typescript-eslint/typescript-estree@8.13.0(typescript@5.6.3)': dependencies: - '@typescript-eslint/types': 8.12.2 - '@typescript-eslint/visitor-keys': 8.12.2 + '@typescript-eslint/types': 8.13.0 + '@typescript-eslint/visitor-keys': 8.13.0 debug: 4.3.7 fast-glob: 3.3.2 is-glob: 4.0.3 @@ -2192,20 +2202,20 @@ snapshots: transitivePeerDependencies: - supports-color - '@typescript-eslint/utils@8.12.2(eslint@9.13.0)(typescript@5.6.3)': + '@typescript-eslint/utils@8.13.0(eslint@9.14.0)(typescript@5.6.3)': dependencies: - '@eslint-community/eslint-utils': 4.4.1(eslint@9.13.0) - '@typescript-eslint/scope-manager': 8.12.2 - '@typescript-eslint/types': 8.12.2 - '@typescript-eslint/typescript-estree': 8.12.2(typescript@5.6.3) - eslint: 9.13.0 + '@eslint-community/eslint-utils': 4.4.1(eslint@9.14.0) + '@typescript-eslint/scope-manager': 8.13.0 + '@typescript-eslint/types': 8.13.0 + '@typescript-eslint/typescript-estree': 8.13.0(typescript@5.6.3) + eslint: 9.14.0 transitivePeerDependencies: - supports-color - typescript - '@typescript-eslint/visitor-keys@8.12.2': + '@typescript-eslint/visitor-keys@8.13.0': dependencies: - '@typescript-eslint/types': 8.12.2 + '@typescript-eslint/types': 8.13.0 eslint-visitor-keys: 3.4.3 acorn-jsx@5.3.2(acorn@8.14.0): @@ -2373,13 +2383,13 @@ snapshots: convert-source-map@2.0.0: {} - create-jest@29.7.0(@types/node@22.8.6): + create-jest@29.7.0(@types/node@22.9.0): dependencies: '@jest/types': 29.6.3 chalk: 4.1.2 exit: 0.1.2 graceful-fs: 4.2.11 - jest-config: 29.7.0(@types/node@22.8.6) + jest-config: 29.7.0(@types/node@22.9.0) jest-util: 29.7.0 prompts: 2.4.2 transitivePeerDependencies: @@ -2394,6 +2404,12 @@ snapshots: shebang-command: 2.0.0 which: 2.0.2 + cross-spawn@7.0.5: + dependencies: + path-key: 3.1.1 + shebang-command: 2.0.0 + which: 2.0.2 + debug@4.3.7: dependencies: ms: 2.1.3 @@ -2428,9 +2444,9 @@ snapshots: escape-string-regexp@4.0.0: {} - eslint-plugin-prettier@5.2.1(eslint@9.13.0)(prettier@3.3.3): + eslint-plugin-prettier@5.2.1(eslint@9.14.0)(prettier@3.3.3): dependencies: - eslint: 9.13.0 + eslint: 9.14.0 prettier: 3.3.3 prettier-linter-helpers: 1.0.0 synckit: 0.9.1 @@ -2444,23 +2460,23 @@ snapshots: eslint-visitor-keys@4.2.0: {} - eslint@9.13.0: + eslint@9.14.0: dependencies: - '@eslint-community/eslint-utils': 4.4.1(eslint@9.13.0) + '@eslint-community/eslint-utils': 4.4.1(eslint@9.14.0) '@eslint-community/regexpp': 4.12.1 '@eslint/config-array': 0.18.0 '@eslint/core': 0.7.0 '@eslint/eslintrc': 3.1.0 - '@eslint/js': 9.13.0 + '@eslint/js': 9.14.0 '@eslint/plugin-kit': 0.2.2 '@humanfs/node': 0.16.6 '@humanwhocodes/module-importer': 1.0.1 - '@humanwhocodes/retry': 0.3.1 + '@humanwhocodes/retry': 0.4.1 '@types/estree': 1.0.6 '@types/json-schema': 7.0.15 ajv: 6.12.6 chalk: 4.1.2 - cross-spawn: 7.0.3 + cross-spawn: 7.0.5 debug: 4.3.7 escape-string-regexp: 4.0.0 eslint-scope: 8.2.0 @@ -2736,7 +2752,7 @@ snapshots: '@jest/expect': 29.7.0 '@jest/test-result': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 chalk: 4.1.2 co: 4.6.0 dedent: 1.5.1 @@ -2756,16 +2772,16 @@ snapshots: - babel-plugin-macros - supports-color - jest-cli@29.7.0(@types/node@22.8.6): + jest-cli@29.7.0(@types/node@22.9.0): dependencies: '@jest/core': 29.7.0 '@jest/test-result': 29.7.0 '@jest/types': 29.6.3 chalk: 4.1.2 - create-jest: 29.7.0(@types/node@22.8.6) + create-jest: 29.7.0(@types/node@22.9.0) exit: 0.1.2 import-local: 3.1.0 - jest-config: 29.7.0(@types/node@22.8.6) + jest-config: 29.7.0(@types/node@22.9.0) jest-util: 29.7.0 jest-validate: 29.7.0 yargs: 17.7.2 @@ -2775,7 +2791,7 @@ snapshots: - supports-color - ts-node - jest-config@29.7.0(@types/node@22.8.6): + jest-config@29.7.0(@types/node@22.9.0): dependencies: '@babel/core': 7.23.9 '@jest/test-sequencer': 29.7.0 @@ -2800,7 +2816,7 @@ snapshots: slash: 3.0.0 strip-json-comments: 3.1.1 optionalDependencies: - '@types/node': 22.8.6 + '@types/node': 22.9.0 transitivePeerDependencies: - babel-plugin-macros - supports-color @@ -2829,7 +2845,7 @@ snapshots: '@jest/environment': 29.7.0 '@jest/fake-timers': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 jest-mock: 29.7.0 jest-util: 29.7.0 @@ -2839,7 +2855,7 @@ snapshots: dependencies: '@jest/types': 29.6.3 '@types/graceful-fs': 4.1.9 - '@types/node': 22.8.6 + '@types/node': 22.9.0 anymatch: 3.1.3 fb-watchman: 2.0.2 graceful-fs: 4.2.11 @@ -2878,7 +2894,7 @@ snapshots: jest-mock@29.7.0: dependencies: '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 jest-util: 29.7.0 jest-pnp-resolver@1.2.3(jest-resolve@29.7.0): @@ -2913,7 +2929,7 @@ snapshots: '@jest/test-result': 29.7.0 '@jest/transform': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 chalk: 4.1.2 emittery: 0.13.1 graceful-fs: 4.2.11 @@ -2941,7 +2957,7 @@ snapshots: '@jest/test-result': 29.7.0 '@jest/transform': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 chalk: 4.1.2 cjs-module-lexer: 1.2.3 collect-v8-coverage: 1.0.2 @@ -2987,7 +3003,7 @@ snapshots: jest-util@29.7.0: dependencies: '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 chalk: 4.1.2 ci-info: 3.9.0 graceful-fs: 4.2.11 @@ -3006,7 +3022,7 @@ snapshots: dependencies: '@jest/test-result': 29.7.0 '@jest/types': 29.6.3 - '@types/node': 22.8.6 + '@types/node': 22.9.0 ansi-escapes: 4.3.2 chalk: 4.1.2 emittery: 0.13.1 @@ -3015,17 +3031,17 @@ snapshots: jest-worker@29.7.0: dependencies: - '@types/node': 22.8.6 + '@types/node': 22.9.0 jest-util: 29.7.0 merge-stream: 2.0.0 supports-color: 8.1.1 - jest@29.7.0(@types/node@22.8.6): + jest@29.7.0(@types/node@22.9.0): dependencies: '@jest/core': 29.7.0 '@jest/types': 29.6.3 import-local: 3.1.0 - jest-cli: 29.7.0(@types/node@22.8.6) + jest-cli: 29.7.0(@types/node@22.9.0) transitivePeerDependencies: - '@types/node' - babel-plugin-macros @@ -3061,7 +3077,7 @@ snapshots: kleur@3.0.3: {} - lemmy-js-client@0.20.0-private-community.9: {} + lemmy-js-client@0.20.0-alpha.18: {} leven@3.1.0: {} @@ -3342,12 +3358,12 @@ snapshots: dependencies: typescript: 5.6.3 - ts-jest@29.2.5(@babel/core@7.23.9)(@jest/transform@29.7.0)(@jest/types@29.6.3)(babel-jest@29.7.0(@babel/core@7.23.9))(jest@29.7.0(@types/node@22.8.6))(typescript@5.6.3): + ts-jest@29.2.5(@babel/core@7.23.9)(@jest/transform@29.7.0)(@jest/types@29.6.3)(babel-jest@29.7.0(@babel/core@7.23.9))(jest@29.7.0(@types/node@22.9.0))(typescript@5.6.3): dependencies: bs-logger: 0.2.6 ejs: 3.1.10 fast-json-stable-stringify: 2.1.0 - jest: 29.7.0(@types/node@22.8.6) + jest: 29.7.0(@types/node@22.9.0) jest-util: 29.7.0 json5: 2.2.3 lodash.memoize: 4.1.2 @@ -3371,11 +3387,11 @@ snapshots: type-fest@0.21.3: {} - typescript-eslint@8.12.2(eslint@9.13.0)(typescript@5.6.3): + typescript-eslint@8.13.0(eslint@9.14.0)(typescript@5.6.3): dependencies: - '@typescript-eslint/eslint-plugin': 8.12.2(@typescript-eslint/parser@8.12.2(eslint@9.13.0)(typescript@5.6.3))(eslint@9.13.0)(typescript@5.6.3) - '@typescript-eslint/parser': 8.12.2(eslint@9.13.0)(typescript@5.6.3) - '@typescript-eslint/utils': 8.12.2(eslint@9.13.0)(typescript@5.6.3) + '@typescript-eslint/eslint-plugin': 8.13.0(@typescript-eslint/parser@8.13.0(eslint@9.14.0)(typescript@5.6.3))(eslint@9.14.0)(typescript@5.6.3) + '@typescript-eslint/parser': 8.13.0(eslint@9.14.0)(typescript@5.6.3) + '@typescript-eslint/utils': 8.13.0(eslint@9.14.0)(typescript@5.6.3) optionalDependencies: typescript: 5.6.3 transitivePeerDependencies: diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 95e916ef2..0b0a9862c 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -9,7 +9,6 @@ import { CreatePrivateMessageReport, DeleteImage, EditCommunity, - GetCommunityPendingFollowsCount, GetCommunityPendingFollowsCountResponse, GetReplies, GetRepliesResponse, @@ -988,7 +987,7 @@ export function getCommentParentId(comment: Comment): number | undefined { if (split.length > 1) { return Number(split[split.length - 2]); } else { - console.log(`Failed to extract comment parent id from ${comment.path}`); + console.error(`Failed to extract comment parent id from ${comment.path}`); return undefined; } } @@ -1006,7 +1005,7 @@ export async function waitUntil( result = await fetcher(); if (checker(result)) return result; } catch (error) { - //console.error(error); + console.error(error); } await delay( delaySeconds[Math.min(retry - 1, delaySeconds.length - 1)] * 1000, diff --git a/crates/api/src/post/hide.rs b/crates/api/src/post/hide.rs index f7c21ef31..58464421c 100644 --- a/crates/api/src/post/hide.rs +++ b/crates/api/src/post/hide.rs @@ -1,34 +1,39 @@ use actix_web::web::{Data, Json}; -use lemmy_api_common::{context::LemmyContext, post::HidePost, SuccessResponse}; +use lemmy_api_common::{ + context::LemmyContext, + post::{HidePost, PostResponse}, +}; use lemmy_db_schema::source::post::PostHide; -use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult, MAX_API_PARAM_ELEMENTS}; -use std::collections::HashSet; +use lemmy_db_views::structs::{LocalUserView, PostView}; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; #[tracing::instrument(skip(context))] pub async fn hide_post( data: Json, context: Data, local_user_view: LocalUserView, -) -> LemmyResult> { - let post_ids = HashSet::from_iter(data.post_ids.clone()); - - if post_ids.len() > MAX_API_PARAM_ELEMENTS { - Err(LemmyErrorType::TooManyItems)?; - } - +) -> LemmyResult> { let person_id = local_user_view.person.id; + let post_id = data.post_id; // Mark the post as hidden / unhidden if data.hide { - PostHide::hide(&mut context.pool(), post_ids, person_id) + PostHide::hide(&mut context.pool(), post_id, person_id) .await .with_lemmy_type(LemmyErrorType::CouldntHidePost)?; } else { - PostHide::unhide(&mut context.pool(), post_ids, person_id) + PostHide::unhide(&mut context.pool(), post_id, person_id) .await .with_lemmy_type(LemmyErrorType::CouldntHidePost)?; } - Ok(Json(SuccessResponse::default())) + let post_view = PostView::read( + &mut context.pool(), + post_id, + Some(&local_user_view.local_user), + false, + ) + .await?; + + Ok(Json(PostResponse { post_view })) } diff --git a/crates/api/src/post/mark_read.rs b/crates/api/src/post/mark_read.rs index 3e534675a..893be56b6 100644 --- a/crates/api/src/post/mark_read.rs +++ b/crates/api/src/post/mark_read.rs @@ -1,34 +1,38 @@ use actix_web::web::{Data, Json}; -use lemmy_api_common::{context::LemmyContext, post::MarkPostAsRead, SuccessResponse}; +use lemmy_api_common::{ + context::LemmyContext, + post::{MarkPostAsRead, PostResponse}, +}; use lemmy_db_schema::source::post::PostRead; -use lemmy_db_views::structs::LocalUserView; -use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult, MAX_API_PARAM_ELEMENTS}; -use std::collections::HashSet; +use lemmy_db_views::structs::{LocalUserView, PostView}; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; #[tracing::instrument(skip(context))] pub async fn mark_post_as_read( data: Json, context: Data, local_user_view: LocalUserView, -) -> LemmyResult> { - let post_ids = HashSet::from_iter(data.post_ids.clone()); - - if post_ids.len() > MAX_API_PARAM_ELEMENTS { - Err(LemmyErrorType::TooManyItems)?; - } - +) -> LemmyResult> { let person_id = local_user_view.person.id; + let post_id = data.post_id; // Mark the post as read / unread if data.read { - PostRead::mark_as_read(&mut context.pool(), post_ids, person_id) + PostRead::mark_as_read(&mut context.pool(), post_id, person_id) .await .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?; } else { - PostRead::mark_as_unread(&mut context.pool(), post_ids, person_id) + PostRead::mark_as_unread(&mut context.pool(), post_id, person_id) .await .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?; } + let post_view = PostView::read( + &mut context.pool(), + post_id, + Some(&local_user_view.local_user), + false, + ) + .await?; - Ok(Json(SuccessResponse::default())) + Ok(Json(PostResponse { post_view })) } diff --git a/crates/api_common/src/post.rs b/crates/api_common/src/post.rs index ca4f53e9d..d5894abd0 100644 --- a/crates/api_common/src/post.rs +++ b/crates/api_common/src/post.rs @@ -193,7 +193,7 @@ pub struct RemovePost { #[cfg_attr(feature = "full", ts(export))] /// Mark a post as read. pub struct MarkPostAsRead { - pub post_ids: Vec, + pub post_id: PostId, pub read: bool, } @@ -203,7 +203,7 @@ pub struct MarkPostAsRead { #[cfg_attr(feature = "full", ts(export))] /// Hide a post from list views pub struct HidePost { - pub post_ids: Vec, + pub post_id: PostId, pub hide: bool, } diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index f2c03509d..a462b7147 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -65,7 +65,7 @@ use lemmy_utils::{ use moka::future::Cache; use regex::{escape, Regex, RegexSet}; use rosetta_i18n::{Language, LanguageId}; -use std::{collections::HashSet, sync::LazyLock}; +use std::sync::LazyLock; use tracing::warn; use url::{ParseError, Url}; use urlencoding::encode; @@ -148,7 +148,7 @@ pub async fn mark_post_as_read( post_id: PostId, pool: &mut DbPool<'_>, ) -> LemmyResult<()> { - PostRead::mark_as_read(pool, HashSet::from([post_id]), person_id) + PostRead::mark_as_read(pool, post_id, person_id) .await .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?; Ok(()) diff --git a/crates/db_schema/src/impls/post.rs b/crates/db_schema/src/impls/post.rs index 5be9d7aae..bf969a50e 100644 --- a/crates/db_schema/src/impls/post.rs +++ b/crates/db_schema/src/impls/post.rs @@ -42,7 +42,6 @@ use diesel::{ TextExpressionMethods, }; use diesel_async::RunQueryDsl; -use std::collections::HashSet; #[async_trait] impl Crud for Post { @@ -335,39 +334,35 @@ impl Saveable for PostSaved { impl PostRead { pub async fn mark_as_read( pool: &mut DbPool<'_>, - post_ids: HashSet, + post_id: PostId, person_id: PersonId, ) -> Result { let conn = &mut get_conn(pool).await?; - let forms = post_ids - .into_iter() - .map(|post_id| { - ( - PostReadForm { post_id, person_id }, - post_actions::read.eq(now().nullable()), - ) - }) - .collect::>(); + let form = ( + &PostReadForm { post_id, person_id }, + post_actions::read.eq(now().nullable()), + ); + insert_into(post_actions::table) - .values(forms) + .values(form) .on_conflict((post_actions::person_id, post_actions::post_id)) .do_update() - .set(post_actions::read.eq(now().nullable())) + .set(form) .execute(conn) .await } pub async fn mark_as_unread( pool: &mut DbPool<'_>, - post_id_: HashSet, + post_id_: PostId, person_id_: PersonId, ) -> Result { let conn = &mut get_conn(pool).await?; uplete::new( post_actions::table - .filter(post_actions::post_id.eq_any(post_id_)) + .filter(post_actions::post_id.eq(post_id_)) .filter(post_actions::person_id.eq(person_id_)), ) .set_null(post_actions::read) @@ -379,39 +374,34 @@ impl PostRead { impl PostHide { pub async fn hide( pool: &mut DbPool<'_>, - post_ids: HashSet, + post_id: PostId, person_id: PersonId, ) -> Result { let conn = &mut get_conn(pool).await?; - let forms = post_ids - .into_iter() - .map(|post_id| { - ( - PostHideForm { post_id, person_id }, - post_actions::hidden.eq(now().nullable()), - ) - }) - .collect::>(); + let form = ( + &PostHideForm { post_id, person_id }, + post_actions::hidden.eq(now().nullable()), + ); insert_into(post_actions::table) - .values(forms) + .values(form) .on_conflict((post_actions::person_id, post_actions::post_id)) .do_update() - .set(post_actions::hidden.eq(now().nullable())) + .set(form) .execute(conn) .await } pub async fn unhide( pool: &mut DbPool<'_>, - post_id_: HashSet, + post_id_: PostId, person_id_: PersonId, ) -> Result { let conn = &mut get_conn(pool).await?; uplete::new( post_actions::table - .filter(post_actions::post_id.eq_any(post_id_)) + .filter(post_actions::post_id.eq(post_id_)) .filter(post_actions::person_id.eq(person_id_)), ) .set_null(post_actions::hidden) @@ -446,7 +436,6 @@ mod tests { use lemmy_utils::error::LemmyResult; use pretty_assertions::assert_eq; use serial_test::serial; - use std::collections::HashSet; use url::Url; #[tokio::test] @@ -547,14 +536,9 @@ mod tests { published: inserted_post_saved.published, }; - // Post Read - let marked_as_read = PostRead::mark_as_read( - pool, - HashSet::from([inserted_post.id, inserted_post2.id]), - inserted_person.id, - ) - .await?; - assert_eq!(2, marked_as_read); + // Mark 2 posts as read + PostRead::mark_as_read(pool, inserted_post.id, inserted_person.id).await?; + PostRead::mark_as_read(pool, inserted_post2.id, inserted_person.id).await?; let read_post = Post::read(pool, inserted_post.id).await?; @@ -572,17 +556,19 @@ mod tests { assert_eq!(uplete::Count::only_updated(1), like_removed); let saved_removed = PostSaved::unsave(pool, &post_saved_form).await?; assert_eq!(uplete::Count::only_updated(1), saved_removed); - let read_removed = PostRead::mark_as_unread( - pool, - HashSet::from([inserted_post.id, inserted_post2.id]), - inserted_person.id, - ) - .await?; - assert_eq!(uplete::Count::only_deleted(2), read_removed); + + let read_removed_1 = + PostRead::mark_as_unread(pool, inserted_post.id, inserted_person.id).await?; + assert_eq!(uplete::Count::only_deleted(1), read_removed_1); + + let read_removed_2 = + PostRead::mark_as_unread(pool, inserted_post2.id, inserted_person.id).await?; + assert_eq!(uplete::Count::only_deleted(1), read_removed_2); let num_deleted = Post::delete(pool, inserted_post.id).await? + Post::delete(pool, inserted_post2.id).await? + Post::delete(pool, inserted_scheduled_post.id).await?; + assert_eq!(3, num_deleted); Community::delete(pool, inserted_community.id).await?; Person::delete(pool, inserted_person.id).await?; diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index 2469422c2..23f3a8134 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -656,10 +656,7 @@ mod tests { use lemmy_utils::error::LemmyResult; use pretty_assertions::assert_eq; use serial_test::serial; - use std::{ - collections::HashSet, - time::{Duration, Instant}, - }; + use std::time::{Duration, Instant}; use url::Url; const POST_WITH_ANOTHER_TITLE: &str = "Another title"; @@ -1526,7 +1523,7 @@ mod tests { // Mark a post as read PostRead::mark_as_read( pool, - HashSet::from([data.inserted_bot_post.id]), + data.inserted_bot_post.id, data.local_user_view.person.id, ) .await?; @@ -1568,7 +1565,7 @@ mod tests { // Mark a post as hidden PostHide::hide( pool, - HashSet::from([data.inserted_bot_post.id]), + data.inserted_bot_post.id, data.local_user_view.person.id, ) .await?; From a9d6d4e6e0f13e8885560135355b0c722a5daf32 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Wed, 13 Nov 2024 10:05:16 -0500 Subject: [PATCH 08/12] Add user setting to auto-mark fetched posts as read. (#5160) * Add user setting to auto-mark fetched posts as read. - Rather than apps collecting up viewed posts ids, and sending many mark as read requests, users can now turn this setting on, and any results from /post/list will be auto-marked as read. - Fixes #5144 * Adding list_post request option to auto-mark as read. * Moving db_perf to before federation tests. * Fixing lemmyerrortype import. * Fixing ts_option. * Fix clippy. * Fix override logic. * Revert "Fix override logic." This reverts commit 923d7f0ecaa3ccc85a62e407082c2f7ea31473fa. * Changing name to mark_as_read --- .woodpecker.yml | 22 +++++++++---------- crates/api/src/local_user/save_settings.rs | 1 + crates/api/src/post/like.rs | 13 ++++------- crates/api/src/post/mark_read.rs | 10 +++------ crates/api/src/post/save.rs | 5 ++--- crates/api_common/src/person.rs | 3 +++ crates/api_common/src/post.rs | 3 +++ crates/api_common/src/utils.rs | 15 +------------ crates/api_crud/src/post/create.rs | 5 ++--- crates/api_crud/src/post/read.rs | 9 +++++--- crates/apub/src/api/list_posts.rs | 19 +++++++++++++++- crates/db_schema/src/impls/post.rs | 7 ++++-- crates/db_schema/src/schema.rs | 1 + crates/db_schema/src/source/local_user.rs | 5 +++++ .../src/registration_application_view.rs | 1 + .../down.sql | 3 +++ .../up.sql | 3 +++ 17 files changed, 72 insertions(+), 53 deletions(-) create mode 100644 migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/down.sql create mode 100644 migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/up.sql diff --git a/.woodpecker.yml b/.woodpecker.yml index 060cc3e26..1a96c2c66 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -138,17 +138,6 @@ steps: - diff tmp.schema crates/db_schema/src/schema.rs when: *slow_check_paths - check_db_perf_tool: - image: *rust_image - environment: - LEMMY_DATABASE_URL: postgres://lemmy:password@database:5432/lemmy - RUST_BACKTRACE: "1" - CARGO_HOME: .cargo_home - commands: - # same as scripts/db_perf.sh but without creating a new database server - - cargo run --package lemmy_db_perf -- --posts 10 --read-post-pages 1 - when: *slow_check_paths - cargo_clippy: image: *rust_image environment: @@ -221,6 +210,17 @@ steps: - diff before.sqldump after.sqldump when: *slow_check_paths + check_db_perf_tool: + image: *rust_image + environment: + LEMMY_DATABASE_URL: postgres://lemmy:password@database:5432/lemmy + RUST_BACKTRACE: "1" + CARGO_HOME: .cargo_home + commands: + # same as scripts/db_perf.sh but without creating a new database server + - cargo run --package lemmy_db_perf -- --posts 10 --read-post-pages 1 + when: *slow_check_paths + run_federation_tests: image: node:22-bookworm-slim environment: diff --git a/crates/api/src/local_user/save_settings.rs b/crates/api/src/local_user/save_settings.rs index ac2e321a1..992fea163 100644 --- a/crates/api/src/local_user/save_settings.rs +++ b/crates/api/src/local_user/save_settings.rs @@ -143,6 +143,7 @@ pub async fn save_user_settings( enable_animated_images: data.enable_animated_images, enable_private_messages: data.enable_private_messages, collapse_bot_comments: data.collapse_bot_comments, + auto_mark_fetched_posts_as_read: data.auto_mark_fetched_posts_as_read, ..Default::default() }; diff --git a/crates/api/src/post/like.rs b/crates/api/src/post/like.rs index ec01e3e8c..e70213765 100644 --- a/crates/api/src/post/like.rs +++ b/crates/api/src/post/like.rs @@ -5,18 +5,12 @@ use lemmy_api_common::{ context::LemmyContext, post::{CreatePostLike, PostResponse}, send_activity::{ActivityChannel, SendActivityData}, - utils::{ - check_bot_account, - check_community_user_action, - check_local_vote_mode, - mark_post_as_read, - VoteItem, - }, + utils::{check_bot_account, check_community_user_action, check_local_vote_mode, VoteItem}, }; use lemmy_db_schema::{ source::{ local_site::LocalSite, - post::{PostLike, PostLikeForm}, + post::{PostLike, PostLikeForm, PostRead}, }, traits::Likeable, }; @@ -72,7 +66,8 @@ pub async fn like_post( .with_lemmy_type(LemmyErrorType::CouldntLikePost)?; } - mark_post_as_read(person_id, post_id, &mut context.pool()).await?; + // Mark Post Read + PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; ActivityChannel::submit_activity( SendActivityData::LikePostOrComment { diff --git a/crates/api/src/post/mark_read.rs b/crates/api/src/post/mark_read.rs index 893be56b6..def7656b1 100644 --- a/crates/api/src/post/mark_read.rs +++ b/crates/api/src/post/mark_read.rs @@ -5,7 +5,7 @@ use lemmy_api_common::{ }; use lemmy_db_schema::source::post::PostRead; use lemmy_db_views::structs::{LocalUserView, PostView}; -use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; +use lemmy_utils::error::LemmyResult; #[tracing::instrument(skip(context))] pub async fn mark_post_as_read( @@ -18,13 +18,9 @@ pub async fn mark_post_as_read( // Mark the post as read / unread if data.read { - PostRead::mark_as_read(&mut context.pool(), post_id, person_id) - .await - .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?; + PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; } else { - PostRead::mark_as_unread(&mut context.pool(), post_id, person_id) - .await - .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?; + PostRead::mark_as_unread(&mut context.pool(), post_id, person_id).await?; } let post_view = PostView::read( &mut context.pool(), diff --git a/crates/api/src/post/save.rs b/crates/api/src/post/save.rs index 4549b62b1..4a382e732 100644 --- a/crates/api/src/post/save.rs +++ b/crates/api/src/post/save.rs @@ -2,10 +2,9 @@ use actix_web::web::{Data, Json}; use lemmy_api_common::{ context::LemmyContext, post::{PostResponse, SavePost}, - utils::mark_post_as_read, }; use lemmy_db_schema::{ - source::post::{PostSaved, PostSavedForm}, + source::post::{PostRead, PostSaved, PostSavedForm}, traits::Saveable, }; use lemmy_db_views::structs::{LocalUserView, PostView}; @@ -42,7 +41,7 @@ pub async fn save_post( ) .await?; - mark_post_as_read(person_id, post_id, &mut context.pool()).await?; + PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; Ok(Json(PostResponse { post_view })) } diff --git a/crates/api_common/src/person.rs b/crates/api_common/src/person.rs index 742dc88db..b95cf5e77 100644 --- a/crates/api_common/src/person.rs +++ b/crates/api_common/src/person.rs @@ -178,6 +178,9 @@ pub struct SaveUserSettings { pub show_downvotes: Option, #[cfg_attr(feature = "full", ts(optional))] pub show_upvote_percentage: Option, + /// Whether to automatically mark fetched posts as read. + #[cfg_attr(feature = "full", ts(optional))] + pub auto_mark_fetched_posts_as_read: Option, } #[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] diff --git a/crates/api_common/src/post.rs b/crates/api_common/src/post.rs index d5894abd0..39e87e683 100644 --- a/crates/api_common/src/post.rs +++ b/crates/api_common/src/post.rs @@ -108,6 +108,9 @@ pub struct GetPosts { /// If true, then show the nsfw posts (even if your user setting is to hide them) #[cfg_attr(feature = "full", ts(optional))] pub show_nsfw: Option, + /// Whether to automatically mark fetched posts as read. + #[cfg_attr(feature = "full", ts(optional))] + pub mark_as_read: Option, #[cfg_attr(feature = "full", ts(optional))] /// If true, then only show posts with no comments pub no_comments_only: Option, diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index a462b7147..09cdac28c 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -28,7 +28,7 @@ use lemmy_db_schema::{ password_reset_request::PasswordResetRequest, person::{Person, PersonUpdateForm}, person_block::PersonBlock, - post::{Post, PostLike, PostRead}, + post::{Post, PostLike}, registration_application::RegistrationApplication, site::Site, }, @@ -141,19 +141,6 @@ pub fn is_top_mod( } } -/// Marks a post as read for a given person. -#[tracing::instrument(skip_all)] -pub async fn mark_post_as_read( - person_id: PersonId, - post_id: PostId, - pool: &mut DbPool<'_>, -) -> LemmyResult<()> { - PostRead::mark_as_read(pool, post_id, person_id) - .await - .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead)?; - Ok(()) -} - /// Updates the read comment count for a post. Usually done when reading or creating a new comment. #[tracing::instrument(skip_all)] pub async fn update_read_comments( diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index 5dee5e21c..6dc847086 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -12,7 +12,6 @@ use lemmy_api_common::{ get_url_blocklist, honeypot_check, local_site_to_slur_regex, - mark_post_as_read, process_markdown_opt, }, }; @@ -21,7 +20,7 @@ use lemmy_db_schema::{ source::{ community::Community, local_site::LocalSite, - post::{Post, PostInsertForm, PostLike, PostLikeForm}, + post::{Post, PostInsertForm, PostLike, PostLikeForm, PostRead}, }, traits::{Crud, Likeable}, utils::diesel_url_create, @@ -153,7 +152,7 @@ pub async fn create_post( .await .with_lemmy_type(LemmyErrorType::CouldntLikePost)?; - mark_post_as_read(person_id, post_id, &mut context.pool()).await?; + PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; build_post_response(&context, community_id, local_user_view, post_id).await } diff --git a/crates/api_crud/src/post/read.rs b/crates/api_crud/src/post/read.rs index 7677d59ef..f3c2e1fa6 100644 --- a/crates/api_crud/src/post/read.rs +++ b/crates/api_crud/src/post/read.rs @@ -2,10 +2,13 @@ use actix_web::web::{Data, Json, Query}; use lemmy_api_common::{ context::LemmyContext, post::{GetPost, GetPostResponse}, - utils::{check_private_instance, is_mod_or_admin_opt, mark_post_as_read, update_read_comments}, + utils::{check_private_instance, is_mod_or_admin_opt, update_read_comments}, }; use lemmy_db_schema::{ - source::{comment::Comment, post::Post}, + source::{ + comment::Comment, + post::{Post, PostRead}, + }, traits::Crud, }; use lemmy_db_views::{ @@ -62,7 +65,7 @@ pub async fn get_post( let post_id = post_view.post.id; if let Some(person_id) = person_id { - mark_post_as_read(person_id, post_id, &mut context.pool()).await?; + PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; update_read_comments( person_id, diff --git a/crates/apub/src/api/list_posts.rs b/crates/apub/src/api/list_posts.rs index cdf24dbaa..dd80dcbcb 100644 --- a/crates/apub/src/api/list_posts.rs +++ b/crates/apub/src/api/list_posts.rs @@ -10,7 +10,10 @@ use lemmy_api_common::{ post::{GetPosts, GetPostsResponse}, utils::{check_conflicting_like_filters, check_private_instance}, }; -use lemmy_db_schema::source::community::Community; +use lemmy_db_schema::{ + newtypes::PostId, + source::{community::Community, post::PostRead}, +}; use lemmy_db_views::{ post_view::PostQuery, structs::{LocalUserView, PaginationCursor, SiteView}, @@ -90,6 +93,20 @@ pub async fn list_posts( .await .with_lemmy_type(LemmyErrorType::CouldntGetPosts)?; + // If in their user settings (or as part of the API request), auto-mark fetched posts as read + if let Some(local_user) = local_user { + if data + .mark_as_read + .unwrap_or(local_user.auto_mark_fetched_posts_as_read) + { + let post_ids = posts.iter().map(|p| p.post.id).collect::>(); + // TODO get rid of this in the next pr + for post_id in post_ids { + PostRead::mark_as_read(&mut context.pool(), post_id, local_user.person_id).await?; + } + } + } + // if this page wasn't empty, then there is a next page after the last post on this page let next_page = posts.last().map(PaginationCursor::after_post); Ok(Json(GetPostsResponse { posts, next_page })) diff --git a/crates/db_schema/src/impls/post.rs b/crates/db_schema/src/impls/post.rs index bf969a50e..c8b34d3bc 100644 --- a/crates/db_schema/src/impls/post.rs +++ b/crates/db_schema/src/impls/post.rs @@ -42,6 +42,7 @@ use diesel::{ TextExpressionMethods, }; use diesel_async::RunQueryDsl; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; #[async_trait] impl Crud for Post { @@ -336,7 +337,7 @@ impl PostRead { pool: &mut DbPool<'_>, post_id: PostId, person_id: PersonId, - ) -> Result { + ) -> LemmyResult { let conn = &mut get_conn(pool).await?; let form = ( @@ -351,13 +352,14 @@ impl PostRead { .set(form) .execute(conn) .await + .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead) } pub async fn mark_as_unread( pool: &mut DbPool<'_>, post_id_: PostId, person_id_: PersonId, - ) -> Result { + ) -> LemmyResult { let conn = &mut get_conn(pool).await?; uplete::new( @@ -368,6 +370,7 @@ impl PostRead { .set_null(post_actions::read) .get_result(conn) .await + .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead) } } diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index 1d6f3f1d0..9e80c4693 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -455,6 +455,7 @@ diesel::table! { enable_private_messages -> Bool, collapse_bot_comments -> Bool, default_comment_sort_type -> CommentSortTypeEnum, + auto_mark_fetched_posts_as_read -> Bool, } } diff --git a/crates/db_schema/src/source/local_user.rs b/crates/db_schema/src/source/local_user.rs index fd15253cc..82d16f7d4 100644 --- a/crates/db_schema/src/source/local_user.rs +++ b/crates/db_schema/src/source/local_user.rs @@ -68,6 +68,8 @@ pub struct LocalUser { /// Whether to auto-collapse bot comments. pub collapse_bot_comments: bool, pub default_comment_sort_type: CommentSortType, + /// Whether to automatically mark fetched posts as read. + pub auto_mark_fetched_posts_as_read: bool, } #[derive(Clone, derive_new::new)] @@ -124,6 +126,8 @@ pub struct LocalUserInsertForm { pub collapse_bot_comments: Option, #[new(default)] pub default_comment_sort_type: Option, + #[new(default)] + pub auto_mark_fetched_posts_as_read: Option, } #[derive(Clone, Default)] @@ -155,4 +159,5 @@ pub struct LocalUserUpdateForm { pub enable_private_messages: Option, pub collapse_bot_comments: Option, pub default_comment_sort_type: Option, + pub auto_mark_fetched_posts_as_read: Option, } diff --git a/crates/db_views/src/registration_application_view.rs b/crates/db_views/src/registration_application_view.rs index b5821ef26..0fa0a5d7e 100644 --- a/crates/db_views/src/registration_application_view.rs +++ b/crates/db_views/src/registration_application_view.rs @@ -242,6 +242,7 @@ mod tests { enable_animated_images: inserted_sara_local_user.enable_animated_images, enable_private_messages: inserted_sara_local_user.enable_private_messages, collapse_bot_comments: inserted_sara_local_user.collapse_bot_comments, + auto_mark_fetched_posts_as_read: false, }, creator: Person { id: inserted_sara_person.id, diff --git a/migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/down.sql b/migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/down.sql new file mode 100644 index 000000000..0f2ff0569 --- /dev/null +++ b/migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE local_user + DROP COLUMN auto_mark_fetched_posts_as_read; + diff --git a/migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/up.sql b/migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/up.sql new file mode 100644 index 000000000..fbe57276b --- /dev/null +++ b/migrations/2024-11-01-233231_add_mark_fetched_posts_as_read/up.sql @@ -0,0 +1,3 @@ +ALTER TABLE local_user + ADD COLUMN auto_mark_fetched_posts_as_read boolean DEFAULT FALSE NOT NULL; + From 7f4e26e29e3b9ffd0da8b7ddbf819c23de604103 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 14 Nov 2024 09:03:39 -0500 Subject: [PATCH 09/12] Add ability to mark multiple posts as read. (#5178) * Removing a few SuccessResponses for PostHide and MarkPostAsRead. - This also removes the pointless multiple post_ids. These can be done as individual calls on the front end anyway. - Fixes #4755 * Fixing federation tests. * Upgrading lemmy-js-client deps. * Add ability to mark several posts as read. Context: - https://github.com/LemmyNet/lemmy/pull/5043 - https://github.com/LemmyNet/lemmy/issues/4755 - https://github.com/LemmyNet/lemmy/pull/5160 * Fix ntfy to notify on success builds also. * Addressing PR comments. --- .woodpecker.yml | 6 ++-- crates/api/src/post/mark_many_read.rs | 24 ++++++++++++++ crates/api/src/post/mod.rs | 1 + crates/api_common/src/post.rs | 9 ++++++ crates/apub/src/api/list_posts.rs | 5 +-- crates/db_schema/src/impls/post.rs | 45 ++++++++++++++++++--------- src/api_routes_http.rs | 2 ++ 7 files changed, 70 insertions(+), 22 deletions(-) create mode 100644 crates/api/src/post/mark_many_read.rs diff --git a/.woodpecker.yml b/.woodpecker.yml index 1a96c2c66..c9ba830dd 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -290,14 +290,14 @@ steps: when: - event: tag - notify_on_failure: + notify_on_build: image: alpine:3 commands: - apk add curl - - "curl -d'Lemmy CI build failed: ${CI_PIPELINE_URL}' ntfy.sh/lemmy_drone_ci" + - "curl -d'Lemmy CI build ${CI_PIPELINE_STATUS}: ${CI_PIPELINE_URL}' ntfy.sh/lemmy_drone_ci" when: - event: [pull_request, tag] - status: failure + status: [failure, success] notify_on_tag_deploy: image: alpine:3 diff --git a/crates/api/src/post/mark_many_read.rs b/crates/api/src/post/mark_many_read.rs new file mode 100644 index 000000000..82c2c0b06 --- /dev/null +++ b/crates/api/src/post/mark_many_read.rs @@ -0,0 +1,24 @@ +use actix_web::web::{Data, Json}; +use lemmy_api_common::{context::LemmyContext, post::MarkManyPostsAsRead, SuccessResponse}; +use lemmy_db_schema::source::post::PostRead; +use lemmy_db_views::structs::LocalUserView; +use lemmy_utils::error::{LemmyErrorType, LemmyResult, MAX_API_PARAM_ELEMENTS}; + +#[tracing::instrument(skip(context))] +pub async fn mark_posts_as_read( + data: Json, + context: Data, + local_user_view: LocalUserView, +) -> LemmyResult> { + let post_ids = &data.post_ids; + if post_ids.len() > MAX_API_PARAM_ELEMENTS { + Err(LemmyErrorType::TooManyItems)?; + } + + let person_id = local_user_view.person.id; + + // Mark the posts as read + PostRead::mark_many_as_read(&mut context.pool(), post_ids, person_id).await?; + + Ok(Json(SuccessResponse::default())) +} diff --git a/crates/api/src/post/mod.rs b/crates/api/src/post/mod.rs index 7287010f7..97410f097 100644 --- a/crates/api/src/post/mod.rs +++ b/crates/api/src/post/mod.rs @@ -4,5 +4,6 @@ pub mod hide; pub mod like; pub mod list_post_likes; pub mod lock; +pub mod mark_many_read; pub mod mark_read; pub mod save; diff --git a/crates/api_common/src/post.rs b/crates/api_common/src/post.rs index 39e87e683..405de3a92 100644 --- a/crates/api_common/src/post.rs +++ b/crates/api_common/src/post.rs @@ -200,6 +200,15 @@ pub struct MarkPostAsRead { pub read: bool, } +#[skip_serializing_none] +#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] +#[cfg_attr(feature = "full", derive(TS))] +#[cfg_attr(feature = "full", ts(export))] +/// Mark several posts as read. +pub struct MarkManyPostsAsRead { + pub post_ids: Vec, +} + #[skip_serializing_none] #[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq, Hash)] #[cfg_attr(feature = "full", derive(TS))] diff --git a/crates/apub/src/api/list_posts.rs b/crates/apub/src/api/list_posts.rs index dd80dcbcb..63e737fdd 100644 --- a/crates/apub/src/api/list_posts.rs +++ b/crates/apub/src/api/list_posts.rs @@ -100,10 +100,7 @@ pub async fn list_posts( .unwrap_or(local_user.auto_mark_fetched_posts_as_read) { let post_ids = posts.iter().map(|p| p.post.id).collect::>(); - // TODO get rid of this in the next pr - for post_id in post_ids { - PostRead::mark_as_read(&mut context.pool(), post_id, local_user.person_id).await?; - } + PostRead::mark_many_as_read(&mut context.pool(), &post_ids, local_user.person_id).await?; } } diff --git a/crates/db_schema/src/impls/post.rs b/crates/db_schema/src/impls/post.rs index c8b34d3bc..e60cd3a2b 100644 --- a/crates/db_schema/src/impls/post.rs +++ b/crates/db_schema/src/impls/post.rs @@ -338,21 +338,7 @@ impl PostRead { post_id: PostId, person_id: PersonId, ) -> LemmyResult { - let conn = &mut get_conn(pool).await?; - - let form = ( - &PostReadForm { post_id, person_id }, - post_actions::read.eq(now().nullable()), - ); - - insert_into(post_actions::table) - .values(form) - .on_conflict((post_actions::person_id, post_actions::post_id)) - .do_update() - .set(form) - .execute(conn) - .await - .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead) + Self::mark_many_as_read(pool, &[post_id], person_id).await } pub async fn mark_as_unread( @@ -372,6 +358,35 @@ impl PostRead { .await .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead) } + + pub async fn mark_many_as_read( + pool: &mut DbPool<'_>, + post_ids: &[PostId], + person_id: PersonId, + ) -> LemmyResult { + let conn = &mut get_conn(pool).await?; + + let forms = post_ids + .iter() + .map(|post_id| { + ( + PostReadForm { + post_id: *post_id, + person_id, + }, + post_actions::read.eq(now().nullable()), + ) + }) + .collect::>(); + insert_into(post_actions::table) + .values(forms) + .on_conflict((post_actions::person_id, post_actions::post_id)) + .do_update() + .set(post_actions::read.eq(now().nullable())) + .execute(conn) + .await + .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead) + } } impl PostHide { diff --git a/src/api_routes_http.rs b/src/api_routes_http.rs index fd65e0671..2f431419c 100644 --- a/src/api_routes_http.rs +++ b/src/api_routes_http.rs @@ -60,6 +60,7 @@ use lemmy_api::{ like::like_post, list_post_likes::list_post_likes, lock::lock_post, + mark_many_read::mark_posts_as_read, mark_read::mark_post_as_read, save::save_post, }, @@ -239,6 +240,7 @@ pub fn config(cfg: &mut web::ServiceConfig, rate_limit: &RateLimitCell) { .route("/delete", web::post().to(delete_post)) .route("/remove", web::post().to(remove_post)) .route("/mark_as_read", web::post().to(mark_post_as_read)) + .route("/mark_many_as_read", web::post().to(mark_posts_as_read)) .route("/hide", web::post().to(hide_post)) .route("/lock", web::post().to(lock_post)) .route("/feature", web::post().to(feature_post)) From 231cce9350389bb6771b5bcf2252fb52438984e4 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Fri, 15 Nov 2024 05:21:08 -0500 Subject: [PATCH 10/12] Cleanup post action forms (#5197) * Removing a few SuccessResponses for PostHide and MarkPostAsRead. - This also removes the pointless multiple post_ids. These can be done as individual calls on the front end anyway. - Fixes #4755 * Fixing federation tests. * Upgrading lemmy-js-client deps. * Add ability to mark several posts as read. Context: - https://github.com/LemmyNet/lemmy/pull/5043 - https://github.com/LemmyNet/lemmy/issues/4755 - https://github.com/LemmyNet/lemmy/pull/5160 * Simplifying forms. * Fixing forms. * Cleanup post action forms by using derive_new defaults. - Fixes #5195 * Fix ntfy to notify on success builds also. * Removing pointless naive_now function. * Running taplo fmt. --- Cargo.toml | 5 +- crates/api/src/post/like.rs | 11 ++- crates/api/src/post/mark_read.rs | 7 +- crates/api/src/post/save.rs | 10 ++- crates/api_crud/src/comment/update.rs | 4 +- crates/api_crud/src/community/update.rs | 5 +- crates/api_crud/src/oauth_provider/update.rs | 5 +- crates/api_crud/src/post/create.rs | 11 ++- crates/api_crud/src/post/read.rs | 5 +- crates/api_crud/src/post/update.rs | 5 +- crates/api_crud/src/private_message/update.rs | 4 +- crates/api_crud/src/site/create.rs | 7 +- crates/api_crud/src/site/update.rs | 7 +- crates/api_crud/src/tagline/update.rs | 4 +- .../apub/src/activities/community/update.rs | 4 +- .../src/activities/create_or_update/post.rs | 6 +- crates/apub/src/activities/voting/mod.rs | 6 +- crates/apub/src/api/user_settings_backup.rs | 5 +- crates/apub/src/objects/comment.rs | 3 +- crates/apub/src/objects/community.rs | 5 +- crates/apub/src/objects/instance.rs | 3 +- crates/apub/src/objects/person.rs | 3 +- crates/apub/src/objects/post.rs | 3 +- crates/apub/src/objects/private_message.rs | 3 +- .../src/aggregates/person_aggregates.rs | 6 +- .../src/aggregates/post_aggregates.rs | 12 +--- crates/db_schema/src/impls/comment.rs | 14 +--- crates/db_schema/src/impls/comment_report.rs | 9 +-- crates/db_schema/src/impls/instance.rs | 4 +- crates/db_schema/src/impls/person.rs | 5 +- crates/db_schema/src/impls/post.rs | 67 +++++++------------ crates/db_schema/src/impls/post_report.rs | 9 +-- .../src/impls/private_message_report.rs | 7 +- crates/db_schema/src/source/comment.rs | 2 +- crates/db_schema/src/source/post.rs | 17 ++++- crates/db_schema/src/utils.rs | 6 +- crates/db_views/src/post_view.rs | 36 +++------- crates/db_views/src/vote_view.rs | 12 +--- crates/federate/src/worker.rs | 4 +- src/code_migrations.rs | 11 +-- src/scheduled_tasks.rs | 12 +--- 41 files changed, 145 insertions(+), 219 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 807e24e3f..79fb5ca1b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -123,7 +123,10 @@ reqwest-tracing = "0.5.3" clokwerk = "0.4.0" doku = { version = "0.21.1", features = ["url-2"] } bcrypt = "0.15.1" -chrono = { version = "0.4.38", features = ["serde"], default-features = false } +chrono = { version = "0.4.38", features = [ + "serde", + "now", +], default-features = false } serde_json = { version = "1.0.121", features = ["preserve_order"] } base64 = "0.22.1" uuid = { version = "1.10.0", features = ["serde", "v4"] } diff --git a/crates/api/src/post/like.rs b/crates/api/src/post/like.rs index e70213765..031e3f0db 100644 --- a/crates/api/src/post/like.rs +++ b/crates/api/src/post/like.rs @@ -10,7 +10,7 @@ use lemmy_api_common::{ use lemmy_db_schema::{ source::{ local_site::LocalSite, - post::{PostLike, PostLikeForm, PostRead}, + post::{PostLike, PostLikeForm, PostRead, PostReadForm}, }, traits::Likeable, }; @@ -47,11 +47,7 @@ pub async fn like_post( ) .await?; - let like_form = PostLikeForm { - post_id: data.post_id, - person_id: local_user_view.person.id, - score: data.score, - }; + let like_form = PostLikeForm::new(data.post_id, local_user_view.person.id, data.score); // Remove any likes first let person_id = local_user_view.person.id; @@ -67,7 +63,8 @@ pub async fn like_post( } // Mark Post Read - PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; + let read_form = PostReadForm::new(post_id, person_id); + PostRead::mark_as_read(&mut context.pool(), &read_form).await?; ActivityChannel::submit_activity( SendActivityData::LikePostOrComment { diff --git a/crates/api/src/post/mark_read.rs b/crates/api/src/post/mark_read.rs index def7656b1..2d3284375 100644 --- a/crates/api/src/post/mark_read.rs +++ b/crates/api/src/post/mark_read.rs @@ -3,7 +3,7 @@ use lemmy_api_common::{ context::LemmyContext, post::{MarkPostAsRead, PostResponse}, }; -use lemmy_db_schema::source::post::PostRead; +use lemmy_db_schema::source::post::{PostRead, PostReadForm}; use lemmy_db_views::structs::{LocalUserView, PostView}; use lemmy_utils::error::LemmyResult; @@ -17,10 +17,11 @@ pub async fn mark_post_as_read( let post_id = data.post_id; // Mark the post as read / unread + let form = PostReadForm::new(post_id, person_id); if data.read { - PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; + PostRead::mark_as_read(&mut context.pool(), &form).await?; } else { - PostRead::mark_as_unread(&mut context.pool(), post_id, person_id).await?; + PostRead::mark_as_unread(&mut context.pool(), &form).await?; } let post_view = PostView::read( &mut context.pool(), diff --git a/crates/api/src/post/save.rs b/crates/api/src/post/save.rs index 4a382e732..cebbd7fd5 100644 --- a/crates/api/src/post/save.rs +++ b/crates/api/src/post/save.rs @@ -4,7 +4,7 @@ use lemmy_api_common::{ post::{PostResponse, SavePost}, }; use lemmy_db_schema::{ - source::post::{PostRead, PostSaved, PostSavedForm}, + source::post::{PostRead, PostReadForm, PostSaved, PostSavedForm}, traits::Saveable, }; use lemmy_db_views::structs::{LocalUserView, PostView}; @@ -16,10 +16,7 @@ pub async fn save_post( context: Data, local_user_view: LocalUserView, ) -> LemmyResult> { - let post_saved_form = PostSavedForm { - post_id: data.post_id, - person_id: local_user_view.person.id, - }; + let post_saved_form = PostSavedForm::new(data.post_id, local_user_view.person.id); if data.save { PostSaved::save(&mut context.pool(), &post_saved_form) @@ -41,7 +38,8 @@ pub async fn save_post( ) .await?; - PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; + let read_form = PostReadForm::new(post_id, person_id); + PostRead::mark_as_read(&mut context.pool(), &read_form).await?; Ok(Json(PostResponse { post_view })) } diff --git a/crates/api_crud/src/comment/update.rs b/crates/api_crud/src/comment/update.rs index 85ee9bff0..1af026204 100644 --- a/crates/api_crud/src/comment/update.rs +++ b/crates/api_crud/src/comment/update.rs @@ -1,5 +1,6 @@ use activitypub_federation::config::Data; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{ build_response::{build_comment_response, send_local_notifs}, comment::{CommentResponse, EditComment}, @@ -19,7 +20,6 @@ use lemmy_db_schema::{ local_site::LocalSite, }, traits::Crud, - utils::naive_now, }; use lemmy_db_views::structs::{CommentView, LocalUserView}; use lemmy_utils::{ @@ -74,7 +74,7 @@ pub async fn update_comment( let form = CommentUpdateForm { content, language_id: Some(language_id), - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), ..Default::default() }; let updated_comment = Comment::update(&mut context.pool(), comment_id, &form) diff --git a/crates/api_crud/src/community/update.rs b/crates/api_crud/src/community/update.rs index 3dca7d892..d9c062c53 100644 --- a/crates/api_crud/src/community/update.rs +++ b/crates/api_crud/src/community/update.rs @@ -1,6 +1,7 @@ use super::check_community_visibility_allowed; use activitypub_federation::config::Data; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{ build_response::build_community_response, community::{CommunityResponse, EditCommunity}, @@ -22,7 +23,7 @@ use lemmy_db_schema::{ local_site::LocalSite, }, traits::Crud, - utils::{diesel_string_update, diesel_url_update, naive_now}, + utils::{diesel_string_update, diesel_url_update}, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::{ @@ -95,7 +96,7 @@ pub async fn update_community( nsfw: data.nsfw, posting_restricted_to_mods: data.posting_restricted_to_mods, visibility: data.visibility, - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), ..Default::default() }; diff --git a/crates/api_crud/src/oauth_provider/update.rs b/crates/api_crud/src/oauth_provider/update.rs index b4734bf36..29ba19b49 100644 --- a/crates/api_crud/src/oauth_provider/update.rs +++ b/crates/api_crud/src/oauth_provider/update.rs @@ -1,10 +1,11 @@ use activitypub_federation::config::Data; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{context::LemmyContext, oauth_provider::EditOAuthProvider, utils::is_admin}; use lemmy_db_schema::{ source::oauth_provider::{OAuthProvider, OAuthProviderUpdateForm}, traits::Crud, - utils::{diesel_required_string_update, diesel_required_url_update, naive_now}, + utils::{diesel_required_string_update, diesel_required_url_update}, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyError; @@ -32,7 +33,7 @@ pub async fn update_oauth_provider( auto_verify_email: data.auto_verify_email, account_linking_enabled: data.account_linking_enabled, enabled: data.enabled, - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), }; let update_result = diff --git a/crates/api_crud/src/post/create.rs b/crates/api_crud/src/post/create.rs index 6dc847086..948a7617e 100644 --- a/crates/api_crud/src/post/create.rs +++ b/crates/api_crud/src/post/create.rs @@ -20,7 +20,7 @@ use lemmy_db_schema::{ source::{ community::Community, local_site::LocalSite, - post::{Post, PostInsertForm, PostLike, PostLikeForm, PostRead}, + post::{Post, PostInsertForm, PostLike, PostLikeForm, PostRead, PostReadForm}, }, traits::{Crud, Likeable}, utils::diesel_url_create, @@ -142,17 +142,14 @@ pub async fn create_post( // They like their own post by default let person_id = local_user_view.person.id; let post_id = inserted_post.id; - let like_form = PostLikeForm { - post_id, - person_id, - score: 1, - }; + let like_form = PostLikeForm::new(post_id, person_id, 1); PostLike::like(&mut context.pool(), &like_form) .await .with_lemmy_type(LemmyErrorType::CouldntLikePost)?; - PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; + let read_form = PostReadForm::new(post_id, person_id); + PostRead::mark_as_read(&mut context.pool(), &read_form).await?; build_post_response(&context, community_id, local_user_view, post_id).await } diff --git a/crates/api_crud/src/post/read.rs b/crates/api_crud/src/post/read.rs index f3c2e1fa6..3b6ef9414 100644 --- a/crates/api_crud/src/post/read.rs +++ b/crates/api_crud/src/post/read.rs @@ -7,7 +7,7 @@ use lemmy_api_common::{ use lemmy_db_schema::{ source::{ comment::Comment, - post::{Post, PostRead}, + post::{Post, PostRead, PostReadForm}, }, traits::Crud, }; @@ -65,7 +65,8 @@ pub async fn get_post( let post_id = post_view.post.id; if let Some(person_id) = person_id { - PostRead::mark_as_read(&mut context.pool(), post_id, person_id).await?; + let read_form = PostReadForm::new(post_id, person_id); + PostRead::mark_as_read(&mut context.pool(), &read_form).await?; update_read_comments( person_id, diff --git a/crates/api_crud/src/post/update.rs b/crates/api_crud/src/post/update.rs index 217a6cc24..24bbed009 100644 --- a/crates/api_crud/src/post/update.rs +++ b/crates/api_crud/src/post/update.rs @@ -1,6 +1,7 @@ use super::{convert_published_time, create::send_webmention}; use activitypub_federation::config::Data; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{ build_response::build_post_response, context::LemmyContext, @@ -22,7 +23,7 @@ use lemmy_db_schema::{ post::{Post, PostUpdateForm}, }, traits::Crud, - utils::{diesel_string_update, diesel_url_update, naive_now}, + utils::{diesel_string_update, diesel_url_update}, }; use lemmy_db_views::structs::{LocalUserView, PostView}; use lemmy_utils::{ @@ -131,7 +132,7 @@ pub async fn update_post( alt_text, nsfw: data.nsfw, language_id: Some(language_id), - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), scheduled_publish_time, ..Default::default() }; diff --git a/crates/api_crud/src/private_message/update.rs b/crates/api_crud/src/private_message/update.rs index aa562c626..b9e4785ef 100644 --- a/crates/api_crud/src/private_message/update.rs +++ b/crates/api_crud/src/private_message/update.rs @@ -1,5 +1,6 @@ use activitypub_federation::config::Data; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{ context::LemmyContext, private_message::{EditPrivateMessage, PrivateMessageResponse}, @@ -12,7 +13,6 @@ use lemmy_db_schema::{ private_message::{PrivateMessage, PrivateMessageUpdateForm}, }, traits::Crud, - utils::naive_now, }; use lemmy_db_views::structs::{LocalUserView, PrivateMessageView}; use lemmy_utils::{ @@ -47,7 +47,7 @@ pub async fn update_private_message( private_message_id, &PrivateMessageUpdateForm { content: Some(content), - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), ..Default::default() }, ) diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index e1ea1d992..b92cf980c 100644 --- a/crates/api_crud/src/site/create.rs +++ b/crates/api_crud/src/site/create.rs @@ -2,6 +2,7 @@ use super::not_zero; use crate::site::{application_question_check, site_default_post_listing_type_check}; use activitypub_federation::{config::Data, http_signatures::generate_actor_keypair}; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{ context::LemmyContext, site::{CreateSite, SiteResponse}, @@ -23,7 +24,7 @@ use lemmy_db_schema::{ site::{Site, SiteUpdateForm}, }, traits::Crud, - utils::{diesel_string_update, diesel_url_create, naive_now}, + utils::{diesel_string_update, diesel_url_create}, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::{ @@ -75,7 +76,7 @@ pub async fn create_site( icon: Some(icon), banner: Some(banner), actor_id: Some(actor_id), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), inbox_url, private_key: Some(Some(keypair.private_key)), public_key: Some(keypair.public_key), @@ -102,7 +103,7 @@ pub async fn create_site( legal_information: diesel_string_update(data.legal_information.as_deref()), application_email_admins: data.application_email_admins, hide_modlog_mod_names: data.hide_modlog_mod_names, - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), slur_filter_regex: diesel_string_update(data.slur_filter_regex.as_deref()), actor_name_max_length: data.actor_name_max_length, federation_enabled: data.federation_enabled, diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index 085ed69d1..5d1e6aa86 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -2,6 +2,7 @@ use super::not_zero; use crate::site::{application_question_check, site_default_post_listing_type_check}; use activitypub_federation::config::Data; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{ context::LemmyContext, request::replace_image, @@ -27,7 +28,7 @@ use lemmy_db_schema::{ site::{Site, SiteUpdateForm}, }, traits::Crud, - utils::{diesel_string_update, diesel_url_update, naive_now}, + utils::{diesel_string_update, diesel_url_update}, RegistrationMode, }; use lemmy_db_views::structs::{LocalUserView, SiteView}; @@ -88,7 +89,7 @@ pub async fn update_site( icon, banner, content_warning: diesel_string_update(data.content_warning.as_deref()), - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), ..Default::default() }; @@ -111,7 +112,7 @@ pub async fn update_site( legal_information: diesel_string_update(data.legal_information.as_deref()), application_email_admins: data.application_email_admins, hide_modlog_mod_names: data.hide_modlog_mod_names, - updated: Some(Some(naive_now())), + updated: Some(Some(Utc::now())), slur_filter_regex: diesel_string_update(data.slur_filter_regex.as_deref()), actor_name_max_length: data.actor_name_max_length, federation_enabled: data.federation_enabled, diff --git a/crates/api_crud/src/tagline/update.rs b/crates/api_crud/src/tagline/update.rs index 043589d26..30b7d4370 100644 --- a/crates/api_crud/src/tagline/update.rs +++ b/crates/api_crud/src/tagline/update.rs @@ -1,5 +1,6 @@ use activitypub_federation::config::Data; use actix_web::web::Json; +use chrono::Utc; use lemmy_api_common::{ context::LemmyContext, tagline::{TaglineResponse, UpdateTagline}, @@ -11,7 +12,6 @@ use lemmy_db_schema::{ tagline::{Tagline, TaglineUpdateForm}, }, traits::Crud, - utils::naive_now, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyError; @@ -33,7 +33,7 @@ pub async fn update_tagline( let tagline_form = TaglineUpdateForm { content, - updated: naive_now(), + updated: Utc::now(), }; let tagline = Tagline::update(&mut context.pool(), data.id, &tagline_form).await?; diff --git a/crates/apub/src/activities/community/update.rs b/crates/apub/src/activities/community/update.rs index 85be94246..fadf918bd 100644 --- a/crates/apub/src/activities/community/update.rs +++ b/crates/apub/src/activities/community/update.rs @@ -17,6 +17,7 @@ use activitypub_federation::{ kinds::activity::UpdateType, traits::{ActivityHandler, Actor, Object}, }; +use chrono::Utc; use lemmy_api_common::context::LemmyContext; use lemmy_db_schema::{ source::{ @@ -25,7 +26,6 @@ use lemmy_db_schema::{ person::Person, }, traits::Crud, - utils::naive_now, }; use lemmy_utils::error::{LemmyError, LemmyResult}; use url::Url; @@ -103,7 +103,7 @@ impl ActivityHandler for UpdateCommunity { nsfw: Some(self.object.sensitive.unwrap_or(false)), actor_id: Some(self.object.id.into()), public_key: Some(self.object.public_key.public_key_pem), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), icon: Some(self.object.icon.map(|i| i.url.into())), banner: Some(self.object.image.map(|i| i.url.into())), followers_url: self.object.followers.map(Into::into), diff --git a/crates/apub/src/activities/create_or_update/post.rs b/crates/apub/src/activities/create_or_update/post.rs index d0cf17a51..832b2da6d 100644 --- a/crates/apub/src/activities/create_or_update/post.rs +++ b/crates/apub/src/activities/create_or_update/post.rs @@ -118,11 +118,7 @@ impl ActivityHandler for CreateOrUpdatePage { let post = ApubPost::from_json(self.object, context).await?; // author likes their own post by default - let like_form = PostLikeForm { - post_id: post.id, - person_id: post.creator_id, - score: 1, - }; + let like_form = PostLikeForm::new(post.id, post.creator_id, 1); PostLike::like(&mut context.pool(), &like_form).await?; // Calculate initial hot_rank for post diff --git a/crates/apub/src/activities/voting/mod.rs b/crates/apub/src/activities/voting/mod.rs index 7c39b2246..5cda291eb 100644 --- a/crates/apub/src/activities/voting/mod.rs +++ b/crates/apub/src/activities/voting/mod.rs @@ -79,11 +79,7 @@ async fn vote_post( context: &Data, ) -> LemmyResult<()> { let post_id = post.id; - let like_form = PostLikeForm { - post_id: post.id, - person_id: actor.id, - score: vote_type.into(), - }; + let like_form = PostLikeForm::new(post.id, actor.id, vote_type.into()); let person_id = actor.id; PostLike::remove(&mut context.pool(), person_id, post_id).await?; PostLike::like(&mut context.pool(), &like_form).await?; diff --git a/crates/apub/src/api/user_settings_backup.rs b/crates/apub/src/api/user_settings_backup.rs index 601ba8664..d5a864bec 100644 --- a/crates/apub/src/api/user_settings_backup.rs +++ b/crates/apub/src/api/user_settings_backup.rs @@ -200,10 +200,7 @@ pub async fn import_settings( &context, |(saved, context)| async move { let post = saved.dereference(&context).await?; - let form = PostSavedForm { - person_id, - post_id: post.id, - }; + let form = PostSavedForm::new(post.id, person_id); PostSaved::save(&mut context.pool(), &form).await?; LemmyResult::Ok(()) }, diff --git a/crates/apub/src/objects/comment.rs b/crates/apub/src/objects/comment.rs index b7c6a5f51..dc0721404 100644 --- a/crates/apub/src/objects/comment.rs +++ b/crates/apub/src/objects/comment.rs @@ -30,7 +30,6 @@ use lemmy_db_schema::{ post::Post, }, traits::Crud, - utils::naive_now, }; use lemmy_utils::{ error::{FederationError, LemmyError, LemmyResult}, @@ -204,7 +203,7 @@ impl Object for ApubComment { language_id, }; let parent_comment_path = parent_comment.map(|t| t.0.path); - let timestamp: DateTime = note.updated.or(note.published).unwrap_or_else(naive_now); + let timestamp: DateTime = note.updated.or(note.published).unwrap_or_else(Utc::now); let comment = Comment::insert_apub( &mut context.pool(), Some(timestamp), diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs index 2a9abc939..689641910 100644 --- a/crates/apub/src/objects/community.rs +++ b/crates/apub/src/objects/community.rs @@ -38,7 +38,6 @@ use lemmy_db_schema::{ local_site::LocalSite, }, traits::{ApubActor, Crud}, - utils::naive_now, CommunityVisibility, }; use lemmy_db_views_actor::structs::CommunityFollowerView; @@ -166,7 +165,7 @@ impl Object for ApubCommunity { nsfw: Some(group.sensitive.unwrap_or(false)), actor_id: Some(group.id.into()), local: Some(false), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), icon, banner, sidebar, @@ -193,7 +192,7 @@ impl Object for ApubCommunity { let languages = LanguageTag::to_language_id_multiple(group.language, &mut context.pool()).await?; - let timestamp = group.updated.or(group.published).unwrap_or_else(naive_now); + let timestamp = group.updated.or(group.published).unwrap_or_else(Utc::now); let community: ApubCommunity = Community::insert_apub(&mut context.pool(), timestamp, &form) .await? .into(); diff --git a/crates/apub/src/objects/instance.rs b/crates/apub/src/objects/instance.rs index a123c85ba..754172fe2 100644 --- a/crates/apub/src/objects/instance.rs +++ b/crates/apub/src/objects/instance.rs @@ -39,7 +39,6 @@ use lemmy_db_schema::{ site::{Site, SiteInsertForm}, }, traits::Crud, - utils::naive_now, }; use lemmy_utils::{ error::{FederationError, LemmyError, LemmyResult}, @@ -163,7 +162,7 @@ impl Object for ApubSite { banner, description: apub.summary, actor_id: Some(apub.id.clone().into()), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), inbox_url: Some(apub.inbox.clone().into()), public_key: Some(apub.public_key.public_key_pem.clone()), private_key: None, diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index 737579662..97b83c194 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -35,7 +35,6 @@ use lemmy_db_schema::{ person::{Person as DbPerson, PersonInsertForm, PersonUpdateForm}, }, traits::{ApubActor, Crud}, - utils::naive_now, }; use lemmy_utils::{ error::{LemmyError, LemmyResult}, @@ -176,7 +175,7 @@ impl Object for ApubPerson { bot_account: Some(person.kind == UserTypes::Service), private_key: None, public_key: person.public_key.public_key_pem, - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), inbox_url: Some( person .endpoints diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index b72fa1728..bcd1dbf8e 100644 --- a/crates/apub/src/objects/post.rs +++ b/crates/apub/src/objects/post.rs @@ -35,7 +35,6 @@ use lemmy_db_schema::{ post::{Post, PostInsertForm, PostUpdateForm}, }, traits::Crud, - utils::naive_now, }; use lemmy_db_views_actor::structs::CommunityModeratorView; use lemmy_utils::{ @@ -260,7 +259,7 @@ impl Object for ApubPost { ..PostInsertForm::new(name, creator.id, community.id) }; - let timestamp = page.updated.or(page.published).unwrap_or_else(naive_now); + let timestamp = page.updated.or(page.published).unwrap_or_else(Utc::now); let post = Post::insert_apub(&mut context.pool(), timestamp, &form).await?; let post_ = post.clone(); let context_ = context.reset_request_count(); diff --git a/crates/apub/src/objects/private_message.rs b/crates/apub/src/objects/private_message.rs index f3a9f140c..9ada5f657 100644 --- a/crates/apub/src/objects/private_message.rs +++ b/crates/apub/src/objects/private_message.rs @@ -31,7 +31,6 @@ use lemmy_db_schema::{ private_message::{PrivateMessage, PrivateMessageInsertForm}, }, traits::Crud, - utils::naive_now, }; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::{ @@ -161,7 +160,7 @@ impl Object for ApubPrivateMessage { ap_id: Some(note.id.into()), local: Some(false), }; - let timestamp = note.updated.or(note.published).unwrap_or_else(naive_now); + let timestamp = note.updated.or(note.published).unwrap_or_else(Utc::now); let pm = PrivateMessage::insert_apub(&mut context.pool(), timestamp, &form).await?; Ok(pm.into()) } diff --git a/crates/db_schema/src/aggregates/person_aggregates.rs b/crates/db_schema/src/aggregates/person_aggregates.rs index 62aa9b609..df7004d0e 100644 --- a/crates/db_schema/src/aggregates/person_aggregates.rs +++ b/crates/db_schema/src/aggregates/person_aggregates.rs @@ -65,11 +65,7 @@ mod tests { ); let inserted_post = Post::create(pool, &new_post).await?; - let post_like = PostLikeForm { - post_id: inserted_post.id, - person_id: inserted_person.id, - score: 1, - }; + let post_like = PostLikeForm::new(inserted_post.id, inserted_person.id, 1); let _inserted_post_like = PostLike::like(pool, &post_like).await?; let comment_form = CommentInsertForm::new( diff --git a/crates/db_schema/src/aggregates/post_aggregates.rs b/crates/db_schema/src/aggregates/post_aggregates.rs index 46747b076..c11ea6e05 100644 --- a/crates/db_schema/src/aggregates/post_aggregates.rs +++ b/crates/db_schema/src/aggregates/post_aggregates.rs @@ -113,11 +113,7 @@ mod tests { let inserted_child_comment = Comment::create(pool, &child_comment_form, Some(&inserted_comment.path)).await?; - let post_like = PostLikeForm { - post_id: inserted_post.id, - person_id: inserted_person.id, - score: 1, - }; + let post_like = PostLikeForm::new(inserted_post.id, inserted_person.id, 1); PostLike::like(pool, &post_like).await?; @@ -129,11 +125,7 @@ mod tests { assert_eq!(0, post_aggs_before_delete.downvotes); // Add a post dislike from the other person - let post_dislike = PostLikeForm { - post_id: inserted_post.id, - person_id: another_inserted_person.id, - score: -1, - }; + let post_dislike = PostLikeForm::new(inserted_post.id, another_inserted_person.id, -1); PostLike::like(pool, &post_dislike).await?; diff --git a/crates/db_schema/src/impls/comment.rs b/crates/db_schema/src/impls/comment.rs index 96ec70fa2..7dcc033a1 100644 --- a/crates/db_schema/src/impls/comment.rs +++ b/crates/db_schema/src/impls/comment.rs @@ -12,15 +12,7 @@ use crate::{ CommentUpdateForm, }, traits::{Crud, Likeable, Saveable}, - utils::{ - functions::coalesce, - get_conn, - naive_now, - now, - uplete, - DbPool, - DELETED_REPLACEMENT_TEXT, - }, + utils::{functions::coalesce, get_conn, now, uplete, DbPool, DELETED_REPLACEMENT_TEXT}, }; use chrono::{DateTime, Utc}; use diesel::{ @@ -46,7 +38,7 @@ impl Comment { .set(( comment::content.eq(DELETED_REPLACEMENT_TEXT), comment::deleted.eq(true), - comment::updated.eq(naive_now()), + comment::updated.eq(Utc::now()), )) .get_results::(conn) .await @@ -61,7 +53,7 @@ impl Comment { diesel::update(comment::table.filter(comment::creator_id.eq(for_creator_id))) .set(( comment::removed.eq(removed), - comment::updated.eq(naive_now()), + comment::updated.eq(Utc::now()), )) .get_results::(conn) .await diff --git a/crates/db_schema/src/impls/comment_report.rs b/crates/db_schema/src/impls/comment_report.rs index 19c12876f..4c6a1e0d0 100644 --- a/crates/db_schema/src/impls/comment_report.rs +++ b/crates/db_schema/src/impls/comment_report.rs @@ -6,8 +6,9 @@ use crate::{ }, source::comment_report::{CommentReport, CommentReportForm}, traits::Reportable, - utils::{get_conn, naive_now, DbPool}, + utils::{get_conn, DbPool}, }; +use chrono::Utc; use diesel::{ dsl::{insert_into, update}, result::Error, @@ -51,7 +52,7 @@ impl Reportable for CommentReport { .set(( resolved.eq(true), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await @@ -67,7 +68,7 @@ impl Reportable for CommentReport { .set(( resolved.eq(true), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await @@ -88,7 +89,7 @@ impl Reportable for CommentReport { .set(( resolved.eq(false), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await diff --git a/crates/db_schema/src/impls/instance.rs b/crates/db_schema/src/impls/instance.rs index 6c72b5e18..d638786fe 100644 --- a/crates/db_schema/src/impls/instance.rs +++ b/crates/db_schema/src/impls/instance.rs @@ -16,11 +16,11 @@ use crate::{ utils::{ functions::{coalesce, lower}, get_conn, - naive_now, now, DbPool, }, }; +use chrono::Utc; use diesel::{ dsl::{count_star, insert_into}, result::Error, @@ -52,7 +52,7 @@ impl Instance { None => { // Instance not in database yet, insert it let form = InstanceForm { - updated: Some(naive_now()), + updated: Some(Utc::now()), ..InstanceForm::new(domain_) }; insert_into(instance::table) diff --git a/crates/db_schema/src/impls/person.rs b/crates/db_schema/src/impls/person.rs index 3ae355b87..fb8c96f04 100644 --- a/crates/db_schema/src/impls/person.rs +++ b/crates/db_schema/src/impls/person.rs @@ -10,8 +10,9 @@ use crate::{ PersonUpdateForm, }, traits::{ApubActor, Crud, Followable}, - utils::{action_query, functions::lower, get_conn, naive_now, now, uplete, DbPool}, + utils::{action_query, functions::lower, get_conn, now, uplete, DbPool}, }; +use chrono::Utc; use diesel::{ dsl::{insert_into, not}, expression::SelectableHelper, @@ -93,7 +94,7 @@ impl Person { person::bio.eq::>(None), person::matrix_user_id.eq::>(None), person::deleted.eq(true), - person::updated.eq(naive_now()), + person::updated.eq(Utc::now()), )) .get_result::(conn) .await diff --git a/crates/db_schema/src/impls/post.rs b/crates/db_schema/src/impls/post.rs index e60cd3a2b..6ecb5cebd 100644 --- a/crates/db_schema/src/impls/post.rs +++ b/crates/db_schema/src/impls/post.rs @@ -1,5 +1,5 @@ use crate::{ - diesel::{BoolExpressionMethods, OptionalExtension}, + diesel::{BoolExpressionMethods, NullableExpressionMethods, OptionalExtension}, newtypes::{CommunityId, DbUrl, PersonId, PostId}, schema::{community, person, post, post_actions}, source::post::{ @@ -19,7 +19,6 @@ use crate::{ utils::{ functions::coalesce, get_conn, - naive_now, now, uplete, DbPool, @@ -37,7 +36,6 @@ use diesel::{ result::Error, DecoratableTarget, ExpressionMethods, - NullableExpressionMethods, QueryDsl, TextExpressionMethods, }; @@ -138,7 +136,7 @@ impl Post { post::url.eq(Option::<&str>::None), post::body.eq(DELETED_REPLACEMENT_TEXT), post::deleted.eq(true), - post::updated.eq(naive_now()), + post::updated.eq(Utc::now()), )) .get_results::(conn) .await @@ -160,7 +158,7 @@ impl Post { } update - .set((post::removed.eq(removed), post::updated.eq(naive_now()))) + .set((post::removed.eq(removed), post::updated.eq(Utc::now()))) .get_results::(conn) .await } @@ -281,7 +279,6 @@ impl Likeable for PostLike { type IdType = PostId; async fn like(pool: &mut DbPool<'_>, post_like_form: &PostLikeForm) -> Result { let conn = &mut get_conn(pool).await?; - let post_like_form = (post_like_form, post_actions::liked.eq(now().nullable())); insert_into(post_actions::table) .values(post_like_form) .on_conflict((post_actions::post_id, post_actions::person_id)) @@ -310,7 +307,6 @@ impl Saveable for PostSaved { type Form = PostSavedForm; async fn save(pool: &mut DbPool<'_>, post_saved_form: &PostSavedForm) -> Result { let conn = &mut get_conn(pool).await?; - let post_saved_form = (post_saved_form, post_actions::saved.eq(now().nullable())); insert_into(post_actions::table) .values(post_saved_form) .on_conflict((post_actions::post_id, post_actions::person_id)) @@ -335,28 +331,25 @@ impl Saveable for PostSaved { impl PostRead { pub async fn mark_as_read( pool: &mut DbPool<'_>, - post_id: PostId, - person_id: PersonId, + post_read_form: &PostReadForm, ) -> LemmyResult { - Self::mark_many_as_read(pool, &[post_id], person_id).await + Self::mark_many_as_read(pool, &[post_read_form.post_id], post_read_form.person_id).await } pub async fn mark_as_unread( pool: &mut DbPool<'_>, - post_id_: PostId, - person_id_: PersonId, - ) -> LemmyResult { + post_read_form: &PostReadForm, + ) -> Result { let conn = &mut get_conn(pool).await?; uplete::new( post_actions::table - .filter(post_actions::post_id.eq(post_id_)) - .filter(post_actions::person_id.eq(person_id_)), + .filter(post_actions::post_id.eq(post_read_form.post_id)) + .filter(post_actions::person_id.eq(post_read_form.person_id)), ) .set_null(post_actions::read) .get_result(conn) .await - .with_lemmy_type(LemmyErrorType::CouldntMarkPostAsRead) } pub async fn mark_many_as_read( @@ -368,16 +361,9 @@ impl PostRead { let forms = post_ids .iter() - .map(|post_id| { - ( - PostReadForm { - post_id: *post_id, - person_id, - }, - post_actions::read.eq(now().nullable()), - ) - }) + .map(|post_id| (PostReadForm::new(*post_id, person_id))) .collect::>(); + insert_into(post_actions::table) .values(forms) .on_conflict((post_actions::person_id, post_actions::post_id)) @@ -397,10 +383,7 @@ impl PostHide { ) -> Result { let conn = &mut get_conn(pool).await?; - let form = ( - &PostHideForm { post_id, person_id }, - post_actions::hidden.eq(now().nullable()), - ); + let form = &PostHideForm::new(post_id, person_id); insert_into(post_actions::table) .values(form) .on_conflict((post_actions::person_id, post_actions::post_id)) @@ -442,6 +425,7 @@ mod tests { PostLike, PostLikeForm, PostRead, + PostReadForm, PostSaved, PostSavedForm, PostUpdateForm, @@ -525,11 +509,7 @@ mod tests { }; // Post Like - let post_like_form = PostLikeForm { - post_id: inserted_post.id, - person_id: inserted_person.id, - score: 1, - }; + let post_like_form = PostLikeForm::new(inserted_post.id, inserted_person.id, 1); let inserted_post_like = PostLike::like(pool, &post_like_form).await?; @@ -541,10 +521,7 @@ mod tests { }; // Post Save - let post_saved_form = PostSavedForm { - post_id: inserted_post.id, - person_id: inserted_person.id, - }; + let post_saved_form = PostSavedForm::new(inserted_post.id, inserted_person.id); let inserted_post_saved = PostSaved::save(pool, &post_saved_form).await?; @@ -555,8 +532,10 @@ mod tests { }; // Mark 2 posts as read - PostRead::mark_as_read(pool, inserted_post.id, inserted_person.id).await?; - PostRead::mark_as_read(pool, inserted_post2.id, inserted_person.id).await?; + let post_read_form_1 = PostReadForm::new(inserted_post.id, inserted_person.id); + PostRead::mark_as_read(pool, &post_read_form_1).await?; + let post_read_form_2 = PostReadForm::new(inserted_post2.id, inserted_person.id); + PostRead::mark_as_read(pool, &post_read_form_2).await?; let read_post = Post::read(pool, inserted_post.id).await?; @@ -575,12 +554,12 @@ mod tests { let saved_removed = PostSaved::unsave(pool, &post_saved_form).await?; assert_eq!(uplete::Count::only_updated(1), saved_removed); - let read_removed_1 = - PostRead::mark_as_unread(pool, inserted_post.id, inserted_person.id).await?; + let read_remove_form_1 = PostReadForm::new(inserted_post.id, inserted_person.id); + let read_removed_1 = PostRead::mark_as_unread(pool, &read_remove_form_1).await?; assert_eq!(uplete::Count::only_deleted(1), read_removed_1); - let read_removed_2 = - PostRead::mark_as_unread(pool, inserted_post2.id, inserted_person.id).await?; + let read_remove_form_2 = PostReadForm::new(inserted_post2.id, inserted_person.id); + let read_removed_2 = PostRead::mark_as_unread(pool, &read_remove_form_2).await?; assert_eq!(uplete::Count::only_deleted(1), read_removed_2); let num_deleted = Post::delete(pool, inserted_post.id).await? diff --git a/crates/db_schema/src/impls/post_report.rs b/crates/db_schema/src/impls/post_report.rs index e7d27aee9..90ac030c1 100644 --- a/crates/db_schema/src/impls/post_report.rs +++ b/crates/db_schema/src/impls/post_report.rs @@ -6,8 +6,9 @@ use crate::{ }, source::post_report::{PostReport, PostReportForm}, traits::Reportable, - utils::{get_conn, naive_now, DbPool}, + utils::{get_conn, DbPool}, }; +use chrono::Utc; use diesel::{ dsl::{insert_into, update}, result::Error, @@ -40,7 +41,7 @@ impl Reportable for PostReport { .set(( resolved.eq(true), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await @@ -56,7 +57,7 @@ impl Reportable for PostReport { .set(( resolved.eq(true), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await @@ -72,7 +73,7 @@ impl Reportable for PostReport { .set(( resolved.eq(false), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await diff --git a/crates/db_schema/src/impls/private_message_report.rs b/crates/db_schema/src/impls/private_message_report.rs index 0d5876659..0a83bf637 100644 --- a/crates/db_schema/src/impls/private_message_report.rs +++ b/crates/db_schema/src/impls/private_message_report.rs @@ -3,8 +3,9 @@ use crate::{ schema::private_message_report::dsl::{private_message_report, resolved, resolver_id, updated}, source::private_message_report::{PrivateMessageReport, PrivateMessageReportForm}, traits::Reportable, - utils::{get_conn, naive_now, DbPool}, + utils::{get_conn, DbPool}, }; +use chrono::Utc; use diesel::{ dsl::{insert_into, update}, result::Error, @@ -40,7 +41,7 @@ impl Reportable for PrivateMessageReport { .set(( resolved.eq(true), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await @@ -65,7 +66,7 @@ impl Reportable for PrivateMessageReport { .set(( resolved.eq(false), resolver_id.eq(by_resolver_id), - updated.eq(naive_now()), + updated.eq(Utc::now()), )) .execute(conn) .await diff --git a/crates/db_schema/src/source/comment.rs b/crates/db_schema/src/source/comment.rs index be9aa7873..d4001807f 100644 --- a/crates/db_schema/src/source/comment.rs +++ b/crates/db_schema/src/source/comment.rs @@ -84,7 +84,7 @@ pub struct CommentInsertForm { pub struct CommentUpdateForm { pub content: Option, pub removed: Option, - // Don't use a default naive_now here, because the create function does a lot of comment updates + // Don't use a default Utc::now here, because the create function does a lot of comment updates pub updated: Option>>, pub deleted: Option, pub ap_id: Option, diff --git a/crates/db_schema/src/source/post.rs b/crates/db_schema/src/source/post.rs index bed659a10..306d79e70 100644 --- a/crates/db_schema/src/source/post.rs +++ b/crates/db_schema/src/source/post.rs @@ -165,7 +165,7 @@ pub struct PostLike { pub published: DateTime, } -#[derive(Clone)] +#[derive(Clone, derive_new::new)] #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] #[cfg_attr(feature = "full", diesel(table_name = post_actions))] pub struct PostLikeForm { @@ -173,6 +173,8 @@ pub struct PostLikeForm { pub person_id: PersonId, #[cfg_attr(feature = "full", diesel(column_name = like_score))] pub score: i16, + #[new(value = "Utc::now()")] + pub liked: DateTime, } #[derive(PartialEq, Eq, Debug)] @@ -192,11 +194,14 @@ pub struct PostSaved { pub published: DateTime, } +#[derive(derive_new::new)] #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] #[cfg_attr(feature = "full", diesel(table_name = post_actions))] pub struct PostSavedForm { pub post_id: PostId, pub person_id: PersonId, + #[new(value = "Utc::now()")] + pub saved: DateTime, } #[derive(PartialEq, Eq, Debug)] @@ -216,11 +221,14 @@ pub struct PostRead { pub published: DateTime, } +#[derive(derive_new::new)] #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] #[cfg_attr(feature = "full", diesel(table_name = post_actions))] -pub(crate) struct PostReadForm { +pub struct PostReadForm { pub post_id: PostId, pub person_id: PersonId, + #[new(value = "Utc::now()")] + pub read: DateTime, } #[derive(PartialEq, Eq, Debug)] @@ -240,9 +248,12 @@ pub struct PostHide { pub published: DateTime, } +#[derive(derive_new::new)] #[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] #[cfg_attr(feature = "full", diesel(table_name = post_actions))] -pub(crate) struct PostHideForm { +pub struct PostHideForm { pub post_id: PostId, pub person_id: PersonId, + #[new(value = "Utc::now()")] + pub hidden: DateTime, } diff --git a/crates/db_schema/src/utils.rs b/crates/db_schema/src/utils.rs index bb7edb13f..791a19f65 100644 --- a/crates/db_schema/src/utils.rs +++ b/crates/db_schema/src/utils.rs @@ -1,7 +1,7 @@ pub mod uplete; use crate::{newtypes::DbUrl, CommentSortType, PostSortType}; -use chrono::{DateTime, TimeDelta, Utc}; +use chrono::TimeDelta; use deadpool::Runtime; use diesel::{ dsl, @@ -499,10 +499,6 @@ pub fn build_db_pool_for_tests() -> ActualDbPool { build_db_pool().expect("db pool missing") } -pub fn naive_now() -> DateTime { - Utc::now() -} - pub fn post_to_comment_sort_type(sort: PostSortType) -> CommentSortType { use PostSortType::*; match sort { diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index 23f3a8134..f408ff0ec 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -641,6 +641,7 @@ mod tests { PostLike, PostLikeForm, PostRead, + PostReadForm, PostSaved, PostSavedForm, PostUpdateForm, @@ -994,11 +995,8 @@ mod tests { let pool = &mut pool.into(); let mut data = init_data(pool).await?; - let post_like_form = PostLikeForm { - post_id: data.inserted_post.id, - person_id: data.local_user_view.person.id, - score: 1, - }; + let post_like_form = + PostLikeForm::new(data.inserted_post.id, data.local_user_view.person.id, 1); let inserted_post_like = PostLike::like(pool, &post_like_form).await?; @@ -1054,18 +1052,12 @@ mod tests { // Like both the bot post, and your own // The liked_only should not show your own post - let post_like_form = PostLikeForm { - post_id: data.inserted_post.id, - person_id: data.local_user_view.person.id, - score: 1, - }; + let post_like_form = + PostLikeForm::new(data.inserted_post.id, data.local_user_view.person.id, 1); PostLike::like(pool, &post_like_form).await?; - let bot_post_like_form = PostLikeForm { - post_id: data.inserted_bot_post.id, - person_id: data.local_user_view.person.id, - score: 1, - }; + let bot_post_like_form = + PostLikeForm::new(data.inserted_bot_post.id, data.local_user_view.person.id, 1); PostLike::like(pool, &bot_post_like_form).await?; // Read the liked only @@ -1103,10 +1095,8 @@ mod tests { // Save only the bot post // The saved_only should only show the bot post - let post_save_form = PostSavedForm { - post_id: data.inserted_bot_post.id, - person_id: data.local_user_view.person.id, - }; + let post_save_form = + PostSavedForm::new(data.inserted_bot_post.id, data.local_user_view.person.id); PostSaved::save(pool, &post_save_form).await?; // Read the saved only @@ -1521,12 +1511,8 @@ mod tests { data.local_user_view.local_user.show_read_posts = false; // Mark a post as read - PostRead::mark_as_read( - pool, - data.inserted_bot_post.id, - data.local_user_view.person.id, - ) - .await?; + let read_form = PostReadForm::new(data.inserted_bot_post.id, data.local_user_view.person.id); + PostRead::mark_as_read(pool, &read_form).await?; // Make sure you don't see the read post in the results let post_listings_hide_read = data.default_post_query().list(&data.site, pool).await?; diff --git a/crates/db_views/src/vote_view.rs b/crates/db_views/src/vote_view.rs index 9af0bd756..827cd3cc9 100644 --- a/crates/db_views/src/vote_view.rs +++ b/crates/db_views/src/vote_view.rs @@ -134,19 +134,11 @@ mod tests { let inserted_comment = Comment::create(pool, &comment_form, None).await?; // Timmy upvotes his own post - let timmy_post_vote_form = PostLikeForm { - post_id: inserted_post.id, - person_id: inserted_timmy.id, - score: 1, - }; + let timmy_post_vote_form = PostLikeForm::new(inserted_post.id, inserted_timmy.id, 1); PostLike::like(pool, &timmy_post_vote_form).await?; // Sara downvotes timmy's post - let sara_post_vote_form = PostLikeForm { - post_id: inserted_post.id, - person_id: inserted_sara.id, - score: -1, - }; + let sara_post_vote_form = PostLikeForm::new(inserted_post.id, inserted_sara.id, -1); PostLike::like(pool, &sara_post_vote_form).await?; let expected_post_vote_views = [ diff --git a/crates/federate/src/worker.rs b/crates/federate/src/worker.rs index b0254ba0b..20cd51d1a 100644 --- a/crates/federate/src/worker.rs +++ b/crates/federate/src/worker.rs @@ -22,7 +22,7 @@ use lemmy_db_schema::{ federation_queue_state::FederationQueueState, instance::{Instance, InstanceForm}, }, - utils::{naive_now, ActualDbPool, DbPool}, + utils::{ActualDbPool, DbPool}, }; use std::{collections::BinaryHeap, ops::Add, time::Duration}; use tokio::{ @@ -291,7 +291,7 @@ impl InstanceWorker { self.instance.updated = Some(Utc::now()); let form = InstanceForm { - updated: Some(naive_now()), + updated: Some(Utc::now()), ..InstanceForm::new(self.instance.domain.clone()) }; Instance::update(&mut self.pool(), self.instance.id, form).await?; diff --git a/src/code_migrations.rs b/src/code_migrations.rs index 84af43ea7..1f7122aa6 100644 --- a/src/code_migrations.rs +++ b/src/code_migrations.rs @@ -1,5 +1,6 @@ // This is for db migrations that require code use activitypub_federation::http_signatures::generate_actor_keypair; +use chrono::Utc; use diesel::{ sql_types::{Nullable, Text}, ExpressionMethods, @@ -26,7 +27,7 @@ use lemmy_db_schema::{ site::{Site, SiteInsertForm, SiteUpdateForm}, }, traits::Crud, - utils::{get_conn, naive_now, DbPool}, + utils::{get_conn, DbPool}, }; use lemmy_utils::{error::LemmyResult, settings::structs::Settings}; use tracing::info; @@ -78,7 +79,7 @@ async fn user_updates_2020_04_02( )?), private_key: Some(Some(keypair.private_key)), public_key: Some(keypair.public_key), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), ..Default::default() }; @@ -118,7 +119,7 @@ async fn community_updates_2020_04_02( actor_id: Some(community_actor_id.clone()), private_key: Some(Some(keypair.private_key)), public_key: Some(keypair.public_key), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), ..Default::default() }; @@ -334,7 +335,7 @@ async fn instance_actor_2022_01_28( let actor_id = Url::parse(protocol_and_hostname)?; let site_form = SiteUpdateForm { actor_id: Some(actor_id.clone().into()), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), inbox_url: Some(generate_inbox_url()?), private_key: Some(Some(key_pair.private_key)), public_key: Some(key_pair.public_key), @@ -465,7 +466,7 @@ async fn initialize_local_site_2022_10_10( .unwrap_or_else(|| "New Site".to_string()); let site_form = SiteInsertForm { actor_id: Some(site_actor_id.clone().into()), - last_refreshed_at: Some(naive_now()), + last_refreshed_at: Some(Utc::now()), inbox_url: Some(generate_inbox_url()?), private_key: Some(site_key_pair.private_key), public_key: Some(site_key_pair.public_key), diff --git a/src/scheduled_tasks.rs b/src/scheduled_tasks.rs index 043d78d6b..dab2cbe3a 100644 --- a/src/scheduled_tasks.rs +++ b/src/scheduled_tasks.rs @@ -36,15 +36,7 @@ use lemmy_db_schema::{ post::{Post, PostUpdateForm}, }, traits::Crud, - utils::{ - find_action, - functions::coalesce, - get_conn, - naive_now, - now, - DbPool, - DELETED_REPLACEMENT_TEXT, - }, + utils::{find_action, functions::coalesce, get_conn, now, DbPool, DELETED_REPLACEMENT_TEXT}, }; use lemmy_routes::nodeinfo::{NodeInfo, NodeInfoWellKnown}; use lemmy_utils::error::LemmyResult; @@ -558,7 +550,7 @@ async fn build_update_instance_form( // Activitypub). That's why we always need to mark instances as updated if they are // alive. let mut instance_form = InstanceForm { - updated: Some(naive_now()), + updated: Some(Utc::now()), ..InstanceForm::new(domain.to_string()) }; From fa4825b5247b1b771d77f6f8a277af4f79c32ffc Mon Sep 17 00:00:00 2001 From: Dessalines Date: Fri, 15 Nov 2024 08:18:52 -0500 Subject: [PATCH 11/12] Removing a few expects from production and test code. (#5193) * Removing a few expects from production and test code. - Fixes #5192 * Using if let filter for admin emails. * Fixing unused error. * Adding expect_used = deny to clippy lints. * Update src/lib.rs Co-authored-by: Nutomic * Update crates/utils/src/settings/structs.rs Co-authored-by: Nutomic * Update crates/utils/src/settings/mod.rs Co-authored-by: Nutomic * Some more cleanup. * Fix clippy --------- Co-authored-by: Nutomic --- Cargo.toml | 1 + .../local_user/change_password_after_reset.rs | 6 +- crates/api/src/local_user/get_captcha.rs | 5 +- crates/api_common/src/context.rs | 1 + crates/api_common/src/request.rs | 8 +- crates/api_common/src/site.rs | 1 + crates/api_common/src/utils.rs | 39 ++++++--- crates/api_crud/src/site/create.rs | 2 +- crates/api_crud/src/site/update.rs | 2 +- crates/api_crud/src/user/create.rs | 39 +++++---- .../apub/src/activities/community/announce.rs | 2 +- crates/apub/src/fetcher/mod.rs | 4 +- crates/apub/src/lib.rs | 13 +-- crates/db_schema/src/impls/actor_language.rs | 5 +- crates/db_schema/src/impls/captcha_answer.rs | 14 ++-- crates/db_schema/src/impls/local_user.rs | 11 +-- crates/db_schema/src/impls/post.rs | 4 +- crates/db_schema/src/source/mod.rs | 1 + crates/db_schema/src/utils.rs | 17 ++-- crates/db_views/src/local_user_view.rs | 8 +- crates/db_views/src/post_view.rs | 1 + crates/db_views_actor/src/community_view.rs | 6 +- crates/federate/src/inboxes.rs | 3 + crates/federate/src/worker.rs | 2 +- crates/routes/src/images.rs | 4 + crates/routes/src/webfinger.rs | 21 +++-- crates/utils/src/email.rs | 16 ++-- crates/utils/src/error.rs | 4 +- crates/utils/src/rate_limit/mod.rs | 2 + crates/utils/src/rate_limit/rate_limiter.rs | 1 + crates/utils/src/request.rs | 1 + crates/utils/src/response.rs | 23 ++--- crates/utils/src/settings/mod.rs | 9 ++ crates/utils/src/settings/structs.rs | 3 +- .../utils/src/utils/markdown/image_links.rs | 10 ++- crates/utils/src/utils/mention.rs | 1 + crates/utils/src/utils/slurs.rs | 23 ++--- crates/utils/src/utils/validation.rs | 84 +++++++++++-------- src/code_migrations.rs | 7 +- src/lib.rs | 7 +- src/main.rs | 4 +- src/scheduled_tasks.rs | 10 +-- 42 files changed, 252 insertions(+), 173 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 79fb5ca1b..e3b042e1b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -79,6 +79,7 @@ unused_self = "deny" unwrap_used = "deny" unimplemented = "deny" unused_async = "deny" +expect_used = "deny" [workspace.dependencies] lemmy_api = { version = "=0.19.6-beta.7", path = "./crates/api" } diff --git a/crates/api/src/local_user/change_password_after_reset.rs b/crates/api/src/local_user/change_password_after_reset.rs index 191815d0f..df99952f4 100644 --- a/crates/api/src/local_user/change_password_after_reset.rs +++ b/crates/api/src/local_user/change_password_after_reset.rs @@ -10,7 +10,7 @@ use lemmy_db_schema::source::{ login_token::LoginToken, password_reset_request::PasswordResetRequest, }; -use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; +use lemmy_utils::error::{LemmyErrorType, LemmyResult}; #[tracing::instrument(skip(context))] pub async fn change_password_after_reset( @@ -32,9 +32,7 @@ pub async fn change_password_after_reset( // Update the user with the new password let password = data.password.clone(); - LocalUser::update_password(&mut context.pool(), local_user_id, &password) - .await - .with_lemmy_type(LemmyErrorType::CouldntUpdateUser)?; + LocalUser::update_password(&mut context.pool(), local_user_id, &password).await?; LoginToken::invalidate_all(&mut context.pool(), local_user_id).await?; diff --git a/crates/api/src/local_user/get_captcha.rs b/crates/api/src/local_user/get_captcha.rs index ac64fa07c..485b95009 100644 --- a/crates/api/src/local_user/get_captcha.rs +++ b/crates/api/src/local_user/get_captcha.rs @@ -12,6 +12,7 @@ use captcha::{gen, Difficulty}; use lemmy_api_common::{ context::LemmyContext, person::{CaptchaResponse, GetCaptchaResponse}, + LemmyErrorType, }; use lemmy_db_schema::source::{ captcha_answer::{CaptchaAnswer, CaptchaAnswerForm}, @@ -37,7 +38,9 @@ pub async fn get_captcha(context: Data) -> LemmyResult FederationConfig { // call this to run migrations let pool = build_db_pool_for_tests(); diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index 96d64d0e5..8ead61c0b 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -521,7 +521,7 @@ mod tests { // root relative url let html_bytes = b""; - let metadata = extract_opengraph_data(html_bytes, &url).expect("Unable to parse metadata"); + let metadata = extract_opengraph_data(html_bytes, &url)?; assert_eq!( metadata.image, Some(Url::parse("https://example.com/image.jpg")?.into()) @@ -529,7 +529,7 @@ mod tests { // base relative url let html_bytes = b""; - let metadata = extract_opengraph_data(html_bytes, &url).expect("Unable to parse metadata"); + let metadata = extract_opengraph_data(html_bytes, &url)?; assert_eq!( metadata.image, Some(Url::parse("https://example.com/one/image.jpg")?.into()) @@ -537,7 +537,7 @@ mod tests { // absolute url let html_bytes = b""; - let metadata = extract_opengraph_data(html_bytes, &url).expect("Unable to parse metadata"); + let metadata = extract_opengraph_data(html_bytes, &url)?; assert_eq!( metadata.image, Some(Url::parse("https://cdn.host.com/image.jpg")?.into()) @@ -545,7 +545,7 @@ mod tests { // protocol relative url let html_bytes = b""; - let metadata = extract_opengraph_data(html_bytes, &url).expect("Unable to parse metadata"); + let metadata = extract_opengraph_data(html_bytes, &url)?; assert_eq!( metadata.image, Some(Url::parse("https://example.com/image.jpg")?.into()) diff --git a/crates/api_common/src/site.rs b/crates/api_common/src/site.rs index 40a5cc42d..91c6151d7 100644 --- a/crates/api_common/src/site.rs +++ b/crates/api_common/src/site.rs @@ -514,6 +514,7 @@ pub struct ReadableFederationState { next_retry: Option>, } +#[allow(clippy::expect_used)] impl From for ReadableFederationState { fn from(internal_state: FederationQueueState) -> Self { ReadableFederationState { diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 09cdac28c..48b339e65 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -442,7 +442,11 @@ pub async fn send_password_reset_email( // Generate a random token let token = uuid::Uuid::new_v4().to_string(); - let email = &user.local_user.email.clone().expect("email"); + let email = &user + .local_user + .email + .clone() + .ok_or(LemmyErrorType::EmailRequired)?; let lang = get_interface_language(user); let subject = &lang.password_reset_subject(&user.person.name); let protocol_and_hostname = settings.get_protocol_and_hostname(); @@ -492,6 +496,7 @@ pub fn get_interface_language_from_settings(user: &LocalUserView) -> Lang { lang_str_to_lang(&user.local_user.interface_language) } +#[allow(clippy::expect_used)] fn lang_str_to_lang(lang: &str) -> Lang { let lang_id = LanguageId::new(lang); Lang::from_language_id(&lang_id).unwrap_or_else(|| { @@ -518,11 +523,11 @@ pub fn local_site_rate_limit_to_rate_limit_config( }) } -pub fn local_site_to_slur_regex(local_site: &LocalSite) -> Option { +pub fn local_site_to_slur_regex(local_site: &LocalSite) -> Option> { build_slur_regex(local_site.slur_filter_regex.as_deref()) } -pub fn local_site_opt_to_slur_regex(local_site: &Option) -> Option { +pub fn local_site_opt_to_slur_regex(local_site: &Option) -> Option> { local_site .as_ref() .map(local_site_to_slur_regex) @@ -557,7 +562,11 @@ pub async fn send_application_approved_email( user: &LocalUserView, settings: &Settings, ) -> LemmyResult<()> { - let email = &user.local_user.email.clone().expect("email"); + let email = &user + .local_user + .email + .clone() + .ok_or(LemmyErrorType::EmailRequired)?; let lang = get_interface_language(user); let subject = lang.registration_approved_subject(&user.person.actor_id); let body = lang.registration_approved_body(&settings.hostname); @@ -579,7 +588,11 @@ pub async fn send_new_applicant_email_to_admins( ); for admin in &admins { - let email = &admin.local_user.email.clone().expect("email"); + let email = &admin + .local_user + .email + .clone() + .ok_or(LemmyErrorType::EmailRequired)?; let lang = get_interface_language_from_settings(admin); let subject = lang.new_application_subject(&settings.hostname, applicant_username); let body = lang.new_application_body(applications_link); @@ -601,11 +614,13 @@ pub async fn send_new_report_email_to_admins( let reports_link = &format!("{}/reports", settings.get_protocol_and_hostname(),); for admin in &admins { - let email = &admin.local_user.email.clone().expect("email"); - let lang = get_interface_language_from_settings(admin); - let subject = lang.new_report_subject(&settings.hostname, reported_username, reporter_username); - let body = lang.new_report_body(reports_link); - send_email(&subject, email, &admin.person.name, &body, settings).await?; + if let Some(email) = &admin.local_user.email { + let lang = get_interface_language_from_settings(admin); + let subject = + lang.new_report_subject(&settings.hostname, reported_username, reporter_username); + let body = lang.new_report_body(reports_link); + send_email(&subject, email, &admin.person.name, &body, settings).await?; + } } Ok(()) } @@ -1030,7 +1045,7 @@ pub fn check_conflicting_like_filters( pub async fn process_markdown( text: &str, - slur_regex: &Option, + slur_regex: &Option>, url_blocklist: &RegexSet, context: &LemmyContext, ) -> LemmyResult { @@ -1062,7 +1077,7 @@ pub async fn process_markdown( pub async fn process_markdown_opt( text: &Option, - slur_regex: &Option, + slur_regex: &Option>, url_blocklist: &RegexSet, context: &LemmyContext, ) -> LemmyResult> { diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index b92cf980c..c8140cc28 100644 --- a/crates/api_crud/src/site/create.rs +++ b/crates/api_crud/src/site/create.rs @@ -162,7 +162,7 @@ fn validate_create_payload(local_site: &LocalSite, create_site: &CreateSite) -> .slur_filter_regex .as_deref() .or(local_site.slur_filter_regex.as_deref()), - )?; + ); site_name_length_check(&create_site.name)?; check_slurs(&create_site.name, &slur_regex)?; diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index 5d1e6aa86..6c23adfb4 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -211,7 +211,7 @@ fn validate_update_payload(local_site: &LocalSite, edit_site: &EditSite) -> Lemm .slur_filter_regex .as_deref() .or(local_site.slur_filter_regex.as_deref()), - )?; + ); if let Some(name) = &edit_site.name { // The name doesn't need to be updated, but if provided it cannot be blanked out... diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index ed560e3d6..14cfc7e44 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -148,14 +148,15 @@ pub async fn register( let inserted_local_user = create_local_user(&context, language_tags, &local_user_form).await?; if local_site.site_setup && require_registration_application { - // Create the registration application - let form = RegistrationApplicationInsertForm { - local_user_id: inserted_local_user.id, - // We already made sure answer was not null above - answer: data.answer.clone().expect("must have an answer"), - }; + if let Some(answer) = data.answer.clone() { + // Create the registration application + let form = RegistrationApplicationInsertForm { + local_user_id: inserted_local_user.id, + answer, + }; - RegistrationApplication::create(&mut context.pool(), &form).await?; + RegistrationApplication::create(&mut context.pool(), &form).await?; + } } // Email the admins, only if email verification is not required @@ -373,17 +374,19 @@ pub async fn authenticate_with_oauth( && !local_user.accepted_application && !local_user.admin { - // Create the registration application - RegistrationApplication::create( - &mut context.pool(), - &RegistrationApplicationInsertForm { - local_user_id: local_user.id, - answer: data.answer.clone().expect("must have an answer"), - }, - ) - .await?; + if let Some(answer) = data.answer.clone() { + // Create the registration application + RegistrationApplication::create( + &mut context.pool(), + &RegistrationApplicationInsertForm { + local_user_id: local_user.id, + answer, + }, + ) + .await?; - login_response.registration_created = true; + login_response.registration_created = true; + } } // Check email is verified when required @@ -483,7 +486,7 @@ async fn send_verification_email_if_required( &local_user .email .clone() - .expect("invalid verification email"), + .ok_or(LemmyErrorType::EmailRequired)?, &mut context.pool(), context.settings(), ) diff --git a/crates/apub/src/activities/community/announce.rs b/crates/apub/src/activities/community/announce.rs index d32b9d76e..b31e4b74f 100644 --- a/crates/apub/src/activities/community/announce.rs +++ b/crates/apub/src/activities/community/announce.rs @@ -130,7 +130,7 @@ impl AnnounceActivity { actor: c.actor.clone().into_inner(), other: serde_json::to_value(c.object)? .as_object() - .expect("is object") + .ok_or(FederationError::Unreachable)? .clone(), }; let announce_compat = AnnounceActivity::new(announcable_page, community, context)?; diff --git a/crates/apub/src/fetcher/mod.rs b/crates/apub/src/fetcher/mod.rs index 29202004f..b2bc35672 100644 --- a/crates/apub/src/fetcher/mod.rs +++ b/crates/apub/src/fetcher/mod.rs @@ -5,7 +5,7 @@ use activitypub_federation::{ }; use diesel::NotFound; use itertools::Itertools; -use lemmy_api_common::context::LemmyContext; +use lemmy_api_common::{context::LemmyContext, LemmyErrorType}; use lemmy_db_schema::traits::ApubActor; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::{LemmyError, LemmyResult}; @@ -42,7 +42,7 @@ where let (name, domain) = identifier .splitn(2, '@') .collect_tuple() - .expect("invalid query"); + .ok_or(LemmyErrorType::InvalidUrl)?; let actor = DbActor::read_from_name_and_domain(&mut context.pool(), name, domain) .await .ok() diff --git a/crates/apub/src/lib.rs b/crates/apub/src/lib.rs index e11475d6c..213f6439d 100644 --- a/crates/apub/src/lib.rs +++ b/crates/apub/src/lib.rs @@ -50,7 +50,8 @@ impl UrlVerifier for VerifyUrlData { async fn verify(&self, url: &Url) -> Result<(), ActivityPubError> { let local_site_data = local_site_data_cached(&mut (&self.0).into()) .await - .expect("read local site data"); + .map_err(|e| ActivityPubError::Other(format!("Cant read local site data: {e}")))?; + use FederationError::*; check_apub_id_valid(url, &local_site_data).map_err(|err| match err { LemmyError { @@ -176,10 +177,7 @@ pub(crate) async fn check_apub_id_valid_with_strictness( .domain() .ok_or(FederationError::UrlWithoutDomain)? .to_string(); - let local_instance = context - .settings() - .get_hostname_without_port() - .expect("local hostname is valid"); + let local_instance = context.settings().get_hostname_without_port()?; if domain == local_instance { return Ok(()); } @@ -196,10 +194,7 @@ pub(crate) async fn check_apub_id_valid_with_strictness( .iter() .map(|i| i.domain.clone()) .collect::>(); - let local_instance = context - .settings() - .get_hostname_without_port() - .expect("local hostname is valid"); + let local_instance = context.settings().get_hostname_without_port()?; allowed_and_local.push(local_instance); let domain = apub_id diff --git a/crates/db_schema/src/impls/actor_language.rs b/crates/db_schema/src/impls/actor_language.rs index 8d4c471be..ab322f0cd 100644 --- a/crates/db_schema/src/impls/actor_language.rs +++ b/crates/db_schema/src/impls/actor_language.rs @@ -372,6 +372,7 @@ async fn convert_update_languages( } /// If all languages are returned, return empty vec instead +#[allow(clippy::expect_used)] async fn convert_read_languages( conn: &mut AsyncPgConnection, language_ids: Vec, @@ -510,7 +511,7 @@ mod tests { #[tokio::test] #[serial] - async fn test_user_languages() -> Result<(), Error> { + async fn test_user_languages() -> LemmyResult<()> { let pool = &build_db_pool_for_tests(); let pool = &mut pool.into(); @@ -543,7 +544,7 @@ mod tests { #[tokio::test] #[serial] - async fn test_community_languages() -> Result<(), Error> { + async fn test_community_languages() -> LemmyResult<()> { let pool = &build_db_pool_for_tests(); let pool = &mut pool.into(); let (site, instance) = create_test_site(pool).await?; diff --git a/crates/db_schema/src/impls/captcha_answer.rs b/crates/db_schema/src/impls/captcha_answer.rs index 8be8fc5de..65e0d36f4 100644 --- a/crates/db_schema/src/impls/captcha_answer.rs +++ b/crates/db_schema/src/impls/captcha_answer.rs @@ -57,11 +57,12 @@ mod tests { source::captcha_answer::{CaptchaAnswer, CaptchaAnswerForm, CheckCaptchaAnswer}, utils::build_db_pool_for_tests, }; + use lemmy_utils::error::LemmyResult; use serial_test::serial; #[tokio::test] #[serial] - async fn test_captcha_happy_path() { + async fn test_captcha_happy_path() -> LemmyResult<()> { let pool = &build_db_pool_for_tests(); let pool = &mut pool.into(); @@ -71,8 +72,7 @@ mod tests { answer: "XYZ".to_string(), }, ) - .await - .expect("should not fail to insert captcha"); + .await?; let result = CaptchaAnswer::check_captcha( pool, @@ -84,11 +84,12 @@ mod tests { .await; assert!(result.is_ok()); + Ok(()) } #[tokio::test] #[serial] - async fn test_captcha_repeat_answer_fails() { + async fn test_captcha_repeat_answer_fails() -> LemmyResult<()> { let pool = &build_db_pool_for_tests(); let pool = &mut pool.into(); @@ -98,8 +99,7 @@ mod tests { answer: "XYZ".to_string(), }, ) - .await - .expect("should not fail to insert captcha"); + .await?; let _result = CaptchaAnswer::check_captcha( pool, @@ -120,5 +120,7 @@ mod tests { .await; assert!(result_repeat.is_err()); + + Ok(()) } } diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index 3b695a97e..6c50e1c94 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -26,19 +26,19 @@ use diesel::{ QueryDsl, }; use diesel_async::RunQueryDsl; -use lemmy_utils::error::{LemmyErrorType, LemmyResult}; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; impl LocalUser { pub async fn create( pool: &mut DbPool<'_>, form: &LocalUserInsertForm, languages: Vec, - ) -> Result { + ) -> LemmyResult { let conn = &mut get_conn(pool).await?; let mut form_with_encrypted_password = form.clone(); if let Some(password_encrypted) = &form.password_encrypted { - let password_hash = hash(password_encrypted, DEFAULT_COST).expect("Couldn't hash password"); + let password_hash = hash(password_encrypted, DEFAULT_COST)?; form_with_encrypted_password.password_encrypted = Some(password_hash); } @@ -84,14 +84,15 @@ impl LocalUser { pool: &mut DbPool<'_>, local_user_id: LocalUserId, new_password: &str, - ) -> Result { + ) -> LemmyResult { let conn = &mut get_conn(pool).await?; - let password_hash = hash(new_password, DEFAULT_COST).expect("Couldn't hash password"); + let password_hash = hash(new_password, DEFAULT_COST)?; diesel::update(local_user::table.find(local_user_id)) .set((local_user::password_encrypted.eq(password_hash),)) .get_result::(conn) .await + .with_lemmy_type(LemmyErrorType::CouldntUpdateUser) } pub async fn set_all_users_email_verified(pool: &mut DbPool<'_>) -> Result, Error> { diff --git a/crates/db_schema/src/impls/post.rs b/crates/db_schema/src/impls/post.rs index 6ecb5cebd..064e21b5f 100644 --- a/crates/db_schema/src/impls/post.rs +++ b/crates/db_schema/src/impls/post.rs @@ -115,9 +115,7 @@ impl Post { .filter(post::local.eq(true)) .filter(post::deleted.eq(false)) .filter(post::removed.eq(false)) - .filter( - post::published.ge(Utc::now().naive_utc() - SITEMAP_DAYS.expect("TimeDelta out of bounds")), - ) + .filter(post::published.ge(Utc::now().naive_utc() - SITEMAP_DAYS)) .order(post::published.desc()) .limit(SITEMAP_LIMIT) .load::<(DbUrl, chrono::DateTime)>(conn) diff --git a/crates/db_schema/src/source/mod.rs b/crates/db_schema/src/source/mod.rs index 377c1aaef..5082ddbd1 100644 --- a/crates/db_schema/src/source/mod.rs +++ b/crates/db_schema/src/source/mod.rs @@ -47,6 +47,7 @@ pub mod tagline; /// This is necessary so they can be successfully deserialized from API responses, even though the /// value is not sent by Lemmy. Necessary for crates which rely on Rust API such as /// lemmy-stats-crawler. +#[allow(clippy::expect_used)] fn placeholder_apub_url() -> DbUrl { DbUrl(Box::new( Url::parse("http://example.com").expect("parse placeholder url"), diff --git a/crates/db_schema/src/utils.rs b/crates/db_schema/src/utils.rs index 791a19f65..ee3e8ce52 100644 --- a/crates/db_schema/src/utils.rs +++ b/crates/db_schema/src/utils.rs @@ -68,7 +68,7 @@ use url::Url; const FETCH_LIMIT_DEFAULT: i64 = 10; pub const FETCH_LIMIT_MAX: i64 = 50; pub const SITEMAP_LIMIT: i64 = 50000; -pub const SITEMAP_DAYS: Option = TimeDelta::try_days(31); +pub const SITEMAP_DAYS: TimeDelta = TimeDelta::days(31); pub const RANK_DEFAULT: f64 = 0.0001; /// Some connection options to speed up queries @@ -360,8 +360,8 @@ pub fn diesel_url_create(opt: Option<&str>) -> LemmyResult> { } /// Sets a few additional config options necessary for starting lemmy -fn build_config_options_uri_segment(config: &str) -> String { - let mut url = Url::parse(config).expect("Couldn't parse postgres connection URI"); +fn build_config_options_uri_segment(config: &str) -> LemmyResult { + let mut url = Url::parse(config)?; // Set `lemmy.protocol_and_hostname` so triggers can use it let lemmy_protocol_and_hostname_option = @@ -377,7 +377,7 @@ fn build_config_options_uri_segment(config: &str) -> String { .join(" "); url.set_query(Some(&format!("options={options_segments}"))); - url.into() + Ok(url.into()) } fn establish_connection(config: &str) -> BoxFuture> { @@ -385,8 +385,11 @@ fn establish_connection(config: &str) -> BoxFuture = OnceLock::new(); - let config = - POSTGRES_CONFIG_WITH_OPTIONS.get_or_init(|| build_config_options_uri_segment(config)); + let config = POSTGRES_CONFIG_WITH_OPTIONS.get_or_init(|| { + build_config_options_uri_segment(config) + .inspect_err(|e| error!("Couldn't parse postgres connection URI: {e}")) + .unwrap_or_default() + }); // We only support TLS with sslmode=require currently let conn = if config.contains("sslmode=require") { @@ -495,6 +498,7 @@ pub fn build_db_pool() -> LemmyResult { Ok(pool) } +#[allow(clippy::expect_used)] pub fn build_db_pool_for_tests() -> ActualDbPool { build_db_pool().expect("db pool missing") } @@ -511,6 +515,7 @@ pub fn post_to_comment_sort_type(sort: PostSortType) -> CommentSortType { } } +#[allow(clippy::expect_used)] static EMAIL_REGEX: LazyLock = LazyLock::new(|| { Regex::new(r"^[a-zA-Z0-9.!#$%&’*+/=?^_`{|}~-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*$") .expect("compile email regex") diff --git a/crates/db_views/src/local_user_view.rs b/crates/db_views/src/local_user_view.rs index 8d55b96fe..68072cb5a 100644 --- a/crates/db_views/src/local_user_view.rs +++ b/crates/db_views/src/local_user_view.rs @@ -20,7 +20,7 @@ use lemmy_db_schema::{ ReadFn, }, }; -use lemmy_utils::error::{LemmyError, LemmyErrorType}; +use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult}; use std::future::{ready, Ready}; enum ReadBy<'a> { @@ -146,7 +146,7 @@ impl LocalUserView { name: &str, bio: &str, admin: bool, - ) -> Result { + ) -> LemmyResult { let instance_id = Instance::read_or_create(pool, "example.com".to_string()) .await? .id; @@ -163,7 +163,9 @@ impl LocalUserView { }; let local_user = LocalUser::create(pool, &user_form, vec![]).await?; - LocalUserView::read(pool, local_user.id).await + LocalUserView::read(pool, local_user.id) + .await + .with_lemmy_type(LemmyErrorType::NotFound) } } diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index f408ff0ec..4363d2a3e 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -501,6 +501,7 @@ pub struct PostQuery<'a> { } impl<'a> PostQuery<'a> { + #[allow(clippy::expect_used)] async fn prefetch_upper_bound_for_page_before( &self, site: &Site, diff --git a/crates/db_views_actor/src/community_view.rs b/crates/db_views_actor/src/community_view.rs index f42340bdb..da349c93e 100644 --- a/crates/db_views_actor/src/community_view.rs +++ b/crates/db_views_actor/src/community_view.rs @@ -286,7 +286,7 @@ mod tests { CommunityVisibility, SubscribedType, }; - use lemmy_utils::error::LemmyResult; + use lemmy_utils::error::{LemmyErrorType, LemmyResult}; use serial_test::serial; use url::Url; @@ -495,7 +495,7 @@ mod tests { }; let communities = query.list(&data.site, pool).await?; for (i, c) in communities.iter().enumerate().skip(1) { - let prev = communities.get(i - 1).expect("No previous community?"); + let prev = communities.get(i - 1).ok_or(LemmyErrorType::NotFound)?; assert!(c.community.title.cmp(&prev.community.title).is_ge()); } @@ -505,7 +505,7 @@ mod tests { }; let communities = query.list(&data.site, pool).await?; for (i, c) in communities.iter().enumerate().skip(1) { - let prev = communities.get(i - 1).expect("No previous community?"); + let prev = communities.get(i - 1).ok_or(LemmyErrorType::NotFound)?; assert!(c.community.title.cmp(&prev.community.title).is_le()); } diff --git a/crates/federate/src/inboxes.rs b/crates/federate/src/inboxes.rs index 1649e019f..33c9c138c 100644 --- a/crates/federate/src/inboxes.rs +++ b/crates/federate/src/inboxes.rs @@ -23,6 +23,7 @@ use std::{ /// currently fairly high because of the current structure of storing inboxes for every person, not /// having a separate list of shared_inboxes, and the architecture of having every instance queue be /// fully separate. (see https://github.com/LemmyNet/lemmy/issues/3958) +#[allow(clippy::expect_used)] static FOLLOW_ADDITIONS_RECHECK_DELAY: LazyLock = LazyLock::new(|| { if *LEMMY_TEST_FAST_FEDERATION { chrono::TimeDelta::try_seconds(1).expect("TimeDelta out of bounds") @@ -33,6 +34,7 @@ static FOLLOW_ADDITIONS_RECHECK_DELAY: LazyLock = LazyLock::n /// The same as FOLLOW_ADDITIONS_RECHECK_DELAY, but triggering when the last person on an instance /// unfollows a specific remote community. This is expected to happen pretty rarely and updating it /// in a timely manner is not too important. +#[allow(clippy::expect_used)] static FOLLOW_REMOVALS_RECHECK_DELAY: LazyLock = LazyLock::new(|| chrono::TimeDelta::try_hours(1).expect("TimeDelta out of bounds")); @@ -472,6 +474,7 @@ mod tests { Ok(()) } + #[allow(clippy::expect_used)] #[tokio::test] async fn test_update_communities() -> LemmyResult<()> { let mut collector = setup_collector(); diff --git a/crates/federate/src/worker.rs b/crates/federate/src/worker.rs index 20cd51d1a..e57cedbd0 100644 --- a/crates/federate/src/worker.rs +++ b/crates/federate/src/worker.rs @@ -331,7 +331,7 @@ impl InstanceWorker { self.state.last_successful_published_time = next.published; } - let save_state_every = chrono::Duration::from_std(SAVE_STATE_EVERY_TIME).expect("not negative"); + let save_state_every = chrono::Duration::from_std(SAVE_STATE_EVERY_TIME)?; if force_write || (Utc::now() - self.last_state_insert) > save_state_every { self.save_and_send_state().await?; } diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index a0f804b6b..54fe4ea83 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -312,12 +312,16 @@ where } // TODO: remove these conversions after actix-web upgrades to http 1.0 +#[allow(clippy::expect_used)] fn convert_status(status: http::StatusCode) -> StatusCode { StatusCode::from_u16(status.as_u16()).expect("status can be converted") } + +#[allow(clippy::expect_used)] fn convert_method(method: &Method) -> http::Method { http::Method::from_bytes(method.as_str().as_bytes()).expect("method can be converted") } + fn convert_header<'a>(name: &'a http::HeaderName, value: &'a HeaderValue) -> (&'a str, &'a [u8]) { (name.as_str(), value.as_bytes()) } diff --git a/crates/routes/src/webfinger.rs b/crates/routes/src/webfinger.rs index c5b7024cd..a20a786f9 100644 --- a/crates/routes/src/webfinger.rs +++ b/crates/routes/src/webfinger.rs @@ -3,13 +3,16 @@ use activitypub_federation::{ fetch::webfinger::{extract_webfinger_name, Webfinger, WebfingerLink, WEBFINGER_CONTENT_TYPE}, }; use actix_web::{web, web::Query, HttpResponse}; -use lemmy_api_common::context::LemmyContext; +use lemmy_api_common::{context::LemmyContext, LemmyErrorType}; use lemmy_db_schema::{ source::{community::Community, person::Person}, traits::ApubActor, CommunityVisibility, }; -use lemmy_utils::{cache_header::cache_3days, error::LemmyResult}; +use lemmy_utils::{ + cache_header::cache_3days, + error::{LemmyErrorExt, LemmyResult}, +}; use serde::Deserialize; use std::collections::HashMap; use url::Url; @@ -41,7 +44,7 @@ async fn get_webfinger_response( let links = if name == context.settings().hostname { // webfinger response for instance actor (required for mastodon authorized fetch) let url = Url::parse(&context.settings().get_protocol_and_hostname())?; - vec![webfinger_link_for_actor(Some(url), "none", &context)] + vec![webfinger_link_for_actor(Some(url), "none", &context)?] } else { // webfinger response for user/community let user_id: Option = Person::read_from_name(&mut context.pool(), name, false) @@ -65,8 +68,8 @@ async fn get_webfinger_response( // Mastodon seems to prioritize the last webfinger item in case of duplicates. Put // community last so that it gets prioritized. For Lemmy the order doesn't matter. vec![ - webfinger_link_for_actor(user_id, "Person", &context), - webfinger_link_for_actor(community_id, "Group", &context), + webfinger_link_for_actor(user_id, "Person", &context)?, + webfinger_link_for_actor(community_id, "Group", &context)?, ] } .into_iter() @@ -94,11 +97,11 @@ fn webfinger_link_for_actor( url: Option, kind: &str, context: &LemmyContext, -) -> Vec { +) -> LemmyResult> { if let Some(url) = url { let type_key = "https://www.w3.org/ns/activitystreams#type" .parse() - .expect("parse url"); + .with_lemmy_type(LemmyErrorType::InvalidUrl)?; let mut vec = vec![ WebfingerLink { @@ -128,8 +131,8 @@ fn webfinger_link_for_actor( ..Default::default() }); } - vec + Ok(vec) } else { - vec![] + Ok(vec![]) } } diff --git a/crates/utils/src/email.rs b/crates/utils/src/email.rs index 7bac7ad67..46abb47ea 100644 --- a/crates/utils/src/email.rs +++ b/crates/utils/src/email.rs @@ -33,7 +33,7 @@ pub async fn send_email( let email_and_port = email_config.smtp_server.split(':').collect::>(); let email = *email_and_port .first() - .ok_or(LemmyErrorType::MissingAnEmail)?; + .ok_or(LemmyErrorType::EmailRequired)?; let port = email_and_port .get(1) .ok_or(LemmyErrorType::EmailSmtpServerNeedsAPort)? @@ -45,16 +45,20 @@ pub async fn send_email( // use usize::MAX as the line wrap length, since lettre handles the wrapping for us let plain_text = html2text::from_read(html.as_bytes(), usize::MAX); + let smtp_from_address = &email_config.smtp_from_address; + let email = Message::builder() .from( - email_config - .smtp_from_address + smtp_from_address .parse() - .expect("email from address isn't valid"), + .with_lemmy_type(LemmyErrorType::InvalidEmailAddress( + smtp_from_address.into(), + ))?, ) .to(Mailbox::new( Some(to_username.to_string()), - Address::from_str(to_email).expect("email to address isn't valid"), + Address::from_str(to_email) + .with_lemmy_type(LemmyErrorType::InvalidEmailAddress(to_email.into()))?, )) .message_id(Some(format!("<{}@{}>", Uuid::new_v4(), settings.hostname))) .subject(subject) @@ -62,7 +66,7 @@ pub async fn send_email( plain_text, html.to_string(), )) - .expect("email built incorrectly"); + .with_lemmy_type(LemmyErrorType::EmailSendFailed)?; // don't worry about 'dangeous'. it's just that leaving it at the default configuration // is bad. diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index d52df2f72..af3b1089e 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -73,7 +73,7 @@ pub enum LemmyErrorType { NoEmailSetup, LocalSiteNotSetup, EmailSmtpServerNeedsAPort, - MissingAnEmail, + InvalidEmailAddress(String), RateLimitError, InvalidName, InvalidDisplayName, @@ -129,6 +129,7 @@ pub enum LemmyErrorType { InvalidRegex, CaptchaIncorrect, CouldntCreateAudioCaptcha, + CouldntCreateImageCaptcha, InvalidUrlScheme, CouldntSendWebmention, ContradictingFilters, @@ -185,6 +186,7 @@ pub enum FederationError { CantDeleteSite, ObjectIsNotPublic, ObjectIsNotPrivate, + Unreachable, } cfg_if! { diff --git a/crates/utils/src/rate_limit/mod.rs b/crates/utils/src/rate_limit/mod.rs index a6cf92150..ace775287 100644 --- a/crates/utils/src/rate_limit/mod.rs +++ b/crates/utils/src/rate_limit/mod.rs @@ -29,6 +29,7 @@ pub struct RateLimitCell { state: Arc>, } +#[allow(clippy::expect_used)] impl RateLimitCell { pub fn new(rate_limit_config: EnumMap) -> Self { let state = Arc::new(Mutex::new(RateLimitState::new(rate_limit_config))); @@ -133,6 +134,7 @@ pub struct RateLimitedMiddleware { service: Rc, } +#[allow(clippy::expect_used)] impl RateLimitChecker { /// Returns true if the request passed the rate limit, false if it failed and should be rejected. pub fn check(self, ip_addr: IpAddr) -> bool { diff --git a/crates/utils/src/rate_limit/rate_limiter.rs b/crates/utils/src/rate_limit/rate_limiter.rs index 01d379986..68f248d6c 100644 --- a/crates/utils/src/rate_limit/rate_limiter.rs +++ b/crates/utils/src/rate_limit/rate_limiter.rs @@ -18,6 +18,7 @@ pub struct InstantSecs { secs: u32, } +#[allow(clippy::expect_used)] impl InstantSecs { pub fn now() -> Self { InstantSecs { diff --git a/crates/utils/src/request.rs b/crates/utils/src/request.rs index e98e0e8a5..5353edb4e 100644 --- a/crates/utils/src/request.rs +++ b/crates/utils/src/request.rs @@ -10,6 +10,7 @@ where } #[tracing::instrument(skip_all)] +#[allow(clippy::expect_used)] async fn retry_custom(f: F) -> Result where F: Fn() -> Fut, diff --git a/crates/utils/src/response.rs b/crates/utils/src/response.rs index f37c15dd7..51ea7198d 100644 --- a/crates/utils/src/response.rs +++ b/crates/utils/src/response.rs @@ -11,19 +11,20 @@ pub fn jsonify_plain_text_errors( return Ok(ErrorHandlerResponse::Response(res.map_into_left_body())); } // We're assuming that any LemmyError is already in JSON format, so we don't need to do anything - if maybe_error - .expect("http responses with 400-599 statuses should have an error object") - .as_error::() - .is_some() - { - return Ok(ErrorHandlerResponse::Response(res.map_into_left_body())); + if let Some(maybe_error) = maybe_error { + if maybe_error.as_error::().is_some() { + return Ok(ErrorHandlerResponse::Response(res.map_into_left_body())); + } } - let (req, res) = res.into_parts(); - let error = res - .error() - .expect("expected an error object in the response"); - let response = HttpResponse::build(res.status()).json(LemmyErrorType::Unknown(error.to_string())); + let (req, res_parts) = res.into_parts(); + let lemmy_err_type = if let Some(error) = res_parts.error() { + LemmyErrorType::Unknown(error.to_string()) + } else { + LemmyErrorType::Unknown("couldnt build json".into()) + }; + + let response = HttpResponse::build(res_parts.status()).json(lemmy_err_type); let service_response = ServiceResponse::new(req, response); Ok(ErrorHandlerResponse::Response( diff --git a/crates/utils/src/settings/mod.rs b/crates/utils/src/settings/mod.rs index aba1a4fb1..986c19059 100644 --- a/crates/utils/src/settings/mod.rs +++ b/crates/utils/src/settings/mod.rs @@ -3,6 +3,7 @@ use anyhow::{anyhow, Context}; use deser_hjson::from_str; use regex::Regex; use std::{env, fs, io::Error, sync::LazyLock}; +use url::Url; use urlencoding::encode; pub mod structs; @@ -11,6 +12,7 @@ use structs::{DatabaseConnection, PictrsConfig, PictrsImageMode, Settings}; static DEFAULT_CONFIG_FILE: &str = "config/config.hjson"; +#[allow(clippy::expect_used)] pub static SETTINGS: LazyLock = LazyLock::new(|| { if env::var("LEMMY_INITIALIZE_WITH_DEFAULT_SETTINGS").is_ok() { println!( @@ -23,6 +25,7 @@ pub static SETTINGS: LazyLock = LazyLock::new(|| { } }); +#[allow(clippy::expect_used)] static WEBFINGER_REGEX: LazyLock = LazyLock::new(|| { Regex::new(&format!( "^acct:([a-zA-Z0-9_]{{3,}})@{}$", @@ -128,3 +131,9 @@ impl PictrsConfig { } } } + +#[allow(clippy::expect_used)] +/// Necessary to avoid URL expect failures +fn pictrs_placeholder_url() -> Url { + Url::parse("http://localhost:8080").expect("parse pictrs url") +} diff --git a/crates/utils/src/settings/structs.rs b/crates/utils/src/settings/structs.rs index e8106d482..fdbec4a95 100644 --- a/crates/utils/src/settings/structs.rs +++ b/crates/utils/src/settings/structs.rs @@ -1,3 +1,4 @@ +use super::pictrs_placeholder_url; use doku::Document; use serde::{Deserialize, Serialize}; use smart_default::SmartDefault; @@ -68,7 +69,7 @@ impl Settings { #[serde(default, deny_unknown_fields)] pub struct PictrsConfig { /// Address where pictrs is available (for image hosting) - #[default(Url::parse("http://localhost:8080").expect("parse pictrs url"))] + #[default(pictrs_placeholder_url())] #[doku(example = "http://localhost:8080")] pub url: Url, diff --git a/crates/utils/src/utils/markdown/image_links.rs b/crates/utils/src/utils/markdown/image_links.rs index 7456190e4..9dcea8da7 100644 --- a/crates/utils/src/utils/markdown/image_links.rs +++ b/crates/utils/src/utils/markdown/image_links.rs @@ -58,11 +58,13 @@ fn find_urls(src: &str) -> Vec<(usize, usize)> { let mut links_offsets = vec![]; ast.walk(|node, _depth| { if let Some(image) = node.cast::() { - let (_, node_offset) = node.srcmap.expect("srcmap is none").get_byte_offsets(); - let start_offset = node_offset - image.url_len() - 1 - image.title_len(); - let end_offset = node_offset - 1; + if let Some(srcmap) = node.srcmap { + let (_, node_offset) = srcmap.get_byte_offsets(); + let start_offset = node_offset - image.url_len() - 1 - image.title_len(); + let end_offset = node_offset - 1; - links_offsets.push((start_offset, end_offset)); + links_offsets.push((start_offset, end_offset)); + } } }); links_offsets diff --git a/crates/utils/src/utils/mention.rs b/crates/utils/src/utils/mention.rs index 13762ed27..0a011f848 100644 --- a/crates/utils/src/utils/mention.rs +++ b/crates/utils/src/utils/mention.rs @@ -2,6 +2,7 @@ use itertools::Itertools; use regex::Regex; use std::sync::LazyLock; +#[allow(clippy::expect_used)] static MENTIONS_REGEX: LazyLock = LazyLock::new(|| { Regex::new(r"@(?P[\w.]+)@(?P[a-zA-Z0-9._:-]+)").expect("compile regex") }); diff --git a/crates/utils/src/utils/slurs.rs b/crates/utils/src/utils/slurs.rs index 2350822eb..8df7bc3d3 100644 --- a/crates/utils/src/utils/slurs.rs +++ b/crates/utils/src/utils/slurs.rs @@ -1,8 +1,8 @@ use crate::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; use regex::{Regex, RegexBuilder}; -pub fn remove_slurs(test: &str, slur_regex: &Option) -> String { - if let Some(slur_regex) = slur_regex { +pub fn remove_slurs(test: &str, slur_regex: &Option>) -> String { + if let Some(Ok(slur_regex)) = slur_regex { slur_regex.replace_all(test, "*removed*").to_string() } else { test.to_string() @@ -11,9 +11,9 @@ pub fn remove_slurs(test: &str, slur_regex: &Option) -> String { pub(crate) fn slur_check<'a>( test: &'a str, - slur_regex: &'a Option, + slur_regex: &'a Option>, ) -> Result<(), Vec<&'a str>> { - if let Some(slur_regex) = slur_regex { + if let Some(Ok(slur_regex)) = slur_regex { let mut matches: Vec<&str> = slur_regex.find_iter(test).map(|mat| mat.as_str()).collect(); // Unique @@ -30,16 +30,16 @@ pub(crate) fn slur_check<'a>( } } -pub fn build_slur_regex(regex_str: Option<&str>) -> Option { +pub fn build_slur_regex(regex_str: Option<&str>) -> Option> { regex_str.map(|slurs| { RegexBuilder::new(slurs) .case_insensitive(true) .build() - .expect("compile regex") + .with_lemmy_type(LemmyErrorType::InvalidRegex) }) } -pub fn check_slurs(text: &str, slur_regex: &Option) -> LemmyResult<()> { +pub fn check_slurs(text: &str, slur_regex: &Option>) -> LemmyResult<()> { if let Err(slurs) = slur_check(text, slur_regex) { Err(anyhow::anyhow!("{}", slurs_vec_to_str(&slurs))).with_lemmy_type(LemmyErrorType::Slurs) } else { @@ -47,7 +47,10 @@ pub fn check_slurs(text: &str, slur_regex: &Option) -> LemmyResult<()> { } } -pub fn check_slurs_opt(text: &Option, slur_regex: &Option) -> LemmyResult<()> { +pub fn check_slurs_opt( + text: &Option, + slur_regex: &Option>, +) -> LemmyResult<()> { match text { Some(t) => check_slurs(t, slur_regex), None => Ok(()), @@ -64,7 +67,7 @@ pub(crate) fn slurs_vec_to_str(slurs: &[&str]) -> String { mod test { use crate::{ - error::LemmyResult, + error::{LemmyErrorExt, LemmyErrorType, LemmyResult}, utils::slurs::{remove_slurs, slur_check, slurs_vec_to_str}, }; use pretty_assertions::assert_eq; @@ -72,7 +75,7 @@ mod test { #[test] fn test_slur_filter() -> LemmyResult<()> { - let slur_regex = Some(RegexBuilder::new(r"(fag(g|got|tard)?\b|cock\s?sucker(s|ing)?|ni((g{2,}|q)+|[gq]{2,})[e3r]+(s|z)?|mudslime?s?|kikes?|\bspi(c|k)s?\b|\bchinks?|gooks?|bitch(es|ing|y)?|whor(es?|ing)|\btr(a|@)nn?(y|ies?)|\b(b|re|r)tard(ed)?s?)").case_insensitive(true).build()?); + let slur_regex = Some(RegexBuilder::new(r"(fag(g|got|tard)?\b|cock\s?sucker(s|ing)?|ni((g{2,}|q)+|[gq]{2,})[e3r]+(s|z)?|mudslime?s?|kikes?|\bspi(c|k)s?\b|\bchinks?|gooks?|bitch(es|ing|y)?|whor(es?|ing)|\btr(a|@)nn?(y|ies?)|\b(b|re|r)tard(ed)?s?)").case_insensitive(true).build().with_lemmy_type(LemmyErrorType::InvalidRegex)); let test = "faggot test kike tranny cocksucker retardeds. Capitalized Niggerz. This is a bunch of other safe text."; let slur_free = "No slurs here"; diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index 43aae8599..17ba27696 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -6,11 +6,13 @@ use std::sync::LazyLock; use url::{ParseError, Url}; // From here: https://github.com/vector-im/element-android/blob/develop/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/MatrixPatterns.kt#L35 +#[allow(clippy::expect_used)] static VALID_MATRIX_ID_REGEX: LazyLock = LazyLock::new(|| { Regex::new(r"^@[A-Za-z0-9\x21-\x39\x3B-\x7F]+:[A-Za-z0-9.-]+(:[0-9]{2,5})?$") .expect("compile regex") }); // taken from https://en.wikipedia.org/wiki/UTM_parameters +#[allow(clippy::expect_used)] static URL_CLEANER: LazyLock = LazyLock::new(|| UrlCleaner::from_embedded_rules().expect("compile clearurls")); const ALLOWED_POST_URL_SCHEMES: [&str; 3] = ["http", "https", "magnet"]; @@ -88,6 +90,7 @@ pub fn is_valid_actor_name(name: &str, actor_name_max_length: usize) -> LemmyRes // Only allow characters from a single alphabet per username. This avoids problems with lookalike // characters like `o` which looks identical in Latin and Cyrillic, and can be used to imitate // other users. Checks for additional alphabets can be added in the same way. + #[allow(clippy::expect_used)] static VALID_ACTOR_NAME_REGEX: LazyLock = LazyLock::new(|| { Regex::new(r"^(?:[a-zA-Z0-9_]+|[0-9_\p{Arabic}]+|[0-9_\p{Cyrillic}]+)$").expect("compile regex") }); @@ -218,33 +221,32 @@ fn min_length_check(item: &str, min_length: usize, min_msg: LemmyErrorType) -> L } /// Attempts to build a regex and check it for common errors before inserting into the DB. -pub fn build_and_check_regex(regex_str_opt: &Option<&str>) -> LemmyResult> { - regex_str_opt.map_or_else( - || Ok(None::), - |regex_str| { - if regex_str.is_empty() { - // If the proposed regex is empty, return as having no regex at all; this is the same - // behavior that happens downstream before the write to the database. - return Ok(None::); - } - - RegexBuilder::new(regex_str) - .case_insensitive(true) - .build() - .with_lemmy_type(LemmyErrorType::InvalidRegex) - .and_then(|regex| { - // NOTE: It is difficult to know, in the universe of user-crafted regex, which ones - // may match against any string text. To keep it simple, we'll match the regex - // against an innocuous string - a single number - which should help catch a regex - // that accidentally matches against all strings. - if regex.is_match("1") { - Err(LemmyErrorType::PermissiveRegex.into()) - } else { - Ok(Some(regex)) - } - }) - }, - ) +pub fn build_and_check_regex(regex_str_opt: &Option<&str>) -> Option> { + if let Some(regex) = regex_str_opt { + if regex.is_empty() { + None + } else { + Some( + RegexBuilder::new(regex) + .case_insensitive(true) + .build() + .with_lemmy_type(LemmyErrorType::InvalidRegex) + .and_then(|regex| { + // NOTE: It is difficult to know, in the universe of user-crafted regex, which ones + // may match against any string text. To keep it simple, we'll match the regex + // against an innocuous string - a single number - which should help catch a regex + // that accidentally matches against all strings. + if regex.is_match("1") { + Err(LemmyErrorType::PermissiveRegex.into()) + } else { + Ok(regex) + } + }), + ) + } + } else { + None + } } /// Cleans a url of tracking parameters. @@ -565,13 +567,27 @@ Line3", #[test] fn test_valid_slur_regex() { - let valid_regexes = [&None, &Some(""), &Some("(foo|bar)")]; + let valid_regex = Some("(foo|bar)"); + let result = build_and_check_regex(&valid_regex); + assert!( + result.is_some_and(|x| x.is_ok()), + "Testing regex: {:?}", + valid_regex + ); + } - valid_regexes.iter().for_each(|regex| { - let result = build_and_check_regex(regex); + #[test] + fn test_missing_slur_regex() { + let missing_regex = None; + let result = build_and_check_regex(&missing_regex); + assert!(result.is_none()); + } - assert!(result.is_ok(), "Testing regex: {:?}", regex); - }); + #[test] + fn test_empty_slur_regex() { + let empty = Some(""); + let result = build_and_check_regex(&empty); + assert!(result.is_none()); } #[test] @@ -587,9 +603,9 @@ Line3", .for_each(|(regex_str, expected_err)| { let result = build_and_check_regex(regex_str); - assert!(result.is_err()); + assert!(result.as_ref().is_some_and(Result::is_err)); assert!( - result.is_err_and(|e| e.error_type.eq(&expected_err.clone())), + result.is_some_and(|x| x.is_err_and(|e| e.error_type.eq(&expected_err.clone()))), "Testing regex {:?}, expected error {}", regex_str, expected_err diff --git a/src/code_migrations.rs b/src/code_migrations.rs index 1f7122aa6..1c23a3f00 100644 --- a/src/code_migrations.rs +++ b/src/code_migrations.rs @@ -29,7 +29,10 @@ use lemmy_db_schema::{ traits::Crud, utils::{get_conn, DbPool}, }; -use lemmy_utils::{error::LemmyResult, settings::structs::Settings}; +use lemmy_utils::{ + error::{LemmyErrorExt, LemmyErrorType, LemmyResult}, + settings::structs::Settings, +}; use tracing::info; use url::Url; @@ -421,7 +424,7 @@ async fn initialize_local_site_2022_10_10( let domain = settings .get_hostname_without_port() - .expect("must have domain"); + .with_lemmy_type(LemmyErrorType::Unknown("must have domain".into()))?; // Upsert this to the instance table let instance = Instance::read_or_create(pool, domain).await?; diff --git a/src/lib.rs b/src/lib.rs index b94b5eab1..7ce4e5503 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -37,7 +37,7 @@ use lemmy_db_schema::{source::secret::Secret, utils::build_db_pool}; use lemmy_federate::{Opts, SendManager}; use lemmy_routes::{feeds, images, nodeinfo, webfinger}; use lemmy_utils::{ - error::LemmyResult, + error::{LemmyErrorType, LemmyResult}, rate_limit::RateLimitCell, response::jsonify_plain_text_errors, settings::{structs::Settings, SETTINGS}, @@ -178,7 +178,8 @@ pub async fn start_lemmy_server(args: CmdArgs) -> LemmyResult<()> { .set(Box::new(move |d, c| { Box::pin(match_outgoing_activities(d, c)) })) - .expect("set function pointer"); + .map_err(|_| LemmyErrorType::Unknown("couldnt set function pointer".into()))?; + let request_data = federation_config.to_request_data(); let outgoing_activities_task = tokio::task::spawn(handle_outgoing_activities( request_data.reset_request_count(), @@ -281,7 +282,7 @@ fn create_http_server( let prom_api_metrics = PrometheusMetricsBuilder::new("lemmy_api") .registry(default_registry().clone()) .build() - .expect("Should always be buildable"); + .map_err(|e| LemmyErrorType::Unknown(format!("Should always be buildable: {e}")))?; let context: LemmyContext = federation_config.deref().clone(); let rate_limit_cell = federation_config.rate_limit_cell().clone(); diff --git a/src/main.rs b/src/main.rs index 6babedff4..6f0497dda 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,6 @@ use clap::Parser; use lemmy_server::{start_lemmy_server, CmdArgs}; -use lemmy_utils::error::LemmyResult; +use lemmy_utils::error::{LemmyErrorType, LemmyResult}; use tracing::level_filters::LevelFilter; use tracing_subscriber::EnvFilter; @@ -17,7 +17,7 @@ pub async fn main() -> LemmyResult<()> { rustls::crypto::ring::default_provider() .install_default() - .expect("Failed to install rustls crypto provider"); + .map_err(|_| LemmyErrorType::Unknown("Failed to install rustls crypto provider".into()))?; start_lemmy_server(args).await?; Ok(()) diff --git a/src/scheduled_tasks.rs b/src/scheduled_tasks.rs index dab2cbe3a..5900fe39f 100644 --- a/src/scheduled_tasks.rs +++ b/src/scheduled_tasks.rs @@ -169,10 +169,7 @@ async fn process_ranks_in_batches( where_clause: &str, set_clause: &str, ) { - let process_start_time: DateTime = Utc - .timestamp_opt(0, 0) - .single() - .expect("0 timestamp creation"); + let process_start_time: DateTime = Utc.timestamp_opt(0, 0).single().unwrap_or_default(); let update_batch_size = 1000; // Bigger batches than this tend to cause seq scans let mut processed_rows_count = 0; @@ -220,10 +217,7 @@ async fn process_ranks_in_batches( /// Post aggregates is a special case, since it needs to join to the community_aggregates /// table, to get the active monthly user counts. async fn process_post_aggregates_ranks_in_batches(conn: &mut AsyncPgConnection) { - let process_start_time: DateTime = Utc - .timestamp_opt(0, 0) - .single() - .expect("0 timestamp creation"); + let process_start_time: DateTime = Utc.timestamp_opt(0, 0).single().unwrap_or_default(); let update_batch_size = 1000; // Bigger batches than this tend to cause seq scans let mut processed_rows_count = 0; From 797aac72810f28125f471d6f7dd1754f26ef700d Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 15 Nov 2024 15:13:43 +0100 Subject: [PATCH 12/12] Check for error when fetching link metadata (fixes #5127) (#5129) * Check for error when fetching link metadata (fixes #5127) * use error_for_status everywhere * dont ignore errors * enable lint * fixes * review * more review --- Cargo.toml | 1 + crates/api/src/lib.rs | 2 +- crates/api/src/local_user/add_admin.rs | 2 +- crates/api/src/local_user/reset_password.rs | 4 +- crates/api_common/src/request.rs | 22 +++--- crates/api_crud/src/user/create.rs | 30 ++++---- .../apub/src/activities/community/report.rs | 3 +- crates/apub/src/http/mod.rs | 6 +- crates/db_schema/src/impls/site.rs | 6 +- .../src/community_follower_view.rs | 5 +- crates/federate/src/inboxes.rs | 75 ++++++------------- crates/federate/src/worker.rs | 28 +++---- crates/utils/src/error.rs | 6 ++ crates/utils/src/utils/validation.rs | 2 +- src/lib.rs | 2 +- src/main.rs | 2 +- 16 files changed, 85 insertions(+), 111 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e3b042e1b..57bf92431 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -79,6 +79,7 @@ unused_self = "deny" unwrap_used = "deny" unimplemented = "deny" unused_async = "deny" +map_err_ignore = "deny" expect_used = "deny" [workspace.dependencies] diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 3ab2ba277..83979212d 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -145,7 +145,7 @@ fn build_totp_2fa(hostname: &str, username: &str, secret: &str) -> LemmyResult LemmyResu // server may ignore this and still respond with the full response .header(RANGE, format!("bytes=0-{}", bytes_to_fetch - 1)) /* -1 because inclusive */ .send() - .await?; + .await? + .error_for_status()?; let content_type: Option = response .headers() @@ -308,7 +309,8 @@ pub async fn purge_image_from_pictrs(image_url: &Url, context: &LemmyContext) -> .timeout(REQWEST_TIMEOUT) .header("x-api-token", pictrs_api_key) .send() - .await?; + .await? + .error_for_status()?; let response: PictrsPurgeResponse = response.json().await.map_err(LemmyError::from)?; @@ -333,8 +335,8 @@ pub async fn delete_image_from_pictrs( .delete(&url) .timeout(REQWEST_TIMEOUT) .send() - .await - .map_err(LemmyError::from)?; + .await? + .error_for_status()?; Ok(()) } @@ -366,6 +368,7 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L .timeout(REQWEST_TIMEOUT) .send() .await? + .error_for_status()? .json::() .await?; @@ -406,16 +409,14 @@ pub async fn fetch_pictrs_proxied_image_details( // Pictrs needs you to fetch the proxied image before you can fetch the details let proxy_url = format!("{pictrs_url}image/original?proxy={encoded_image_url}"); - let res = context + context .client() .get(&proxy_url) .timeout(REQWEST_TIMEOUT) .send() .await? - .status(); - if !res.is_success() { - Err(LemmyErrorType::NotAnImageType)? - } + .error_for_status() + .with_lemmy_type(LemmyErrorType::NotAnImageType)?; let details_url = format!("{pictrs_url}image/details/original?proxy={encoded_image_url}"); @@ -425,6 +426,7 @@ pub async fn fetch_pictrs_proxied_image_details( .timeout(REQWEST_TIMEOUT) .send() .await? + .error_for_status()? .json() .await?; diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 14cfc7e44..3bd372937 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -305,7 +305,7 @@ pub async fn authenticate_with_oauth( OAuthAccount::create(&mut context.pool(), &oauth_account_form) .await - .map_err(|_| LemmyErrorType::OauthLoginFailed)?; + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)?; local_user = user_view.local_user.clone(); } else { @@ -366,7 +366,7 @@ pub async fn authenticate_with_oauth( OAuthAccount::create(&mut context.pool(), &oauth_account_form) .await - .map_err(|_| LemmyErrorType::IncorrectLogin)?; + .with_lemmy_type(LemmyErrorType::IncorrectLogin)?; // prevent sign in until application is accepted if local_site.site_setup @@ -527,18 +527,16 @@ async fn oauth_request_access_token( ("client_secret", &oauth_provider.client_secret), ]) .send() - .await; - - let response = response.map_err(|_| LemmyErrorType::OauthLoginFailed)?; - if !response.status().is_success() { - Err(LemmyErrorType::OauthLoginFailed)?; - } + .await + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)? + .error_for_status() + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)?; // Extract the access token let token_response = response .json::() .await - .map_err(|_| LemmyErrorType::OauthLoginFailed)?; + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)?; Ok(token_response) } @@ -555,18 +553,16 @@ async fn oidc_get_user_info( .header("Accept", "application/json") .bearer_auth(access_token) .send() - .await; - - let response = response.map_err(|_| LemmyErrorType::OauthLoginFailed)?; - if !response.status().is_success() { - Err(LemmyErrorType::OauthLoginFailed)?; - } + .await + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)? + .error_for_status() + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)?; // Extract the OAUTH user_id claim from the returned user_info let user_info = response .json::() .await - .map_err(|_| LemmyErrorType::OauthLoginFailed)?; + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)?; Ok(user_info) } @@ -574,7 +570,7 @@ async fn oidc_get_user_info( fn read_user_info(user_info: &serde_json::Value, key: &str) -> LemmyResult { if let Some(value) = user_info.get(key) { let result = serde_json::from_value::(value.clone()) - .map_err(|_| LemmyErrorType::OauthLoginFailed)?; + .with_lemmy_type(LemmyErrorType::OauthLoginFailed)?; return Ok(result); } Err(LemmyErrorType::OauthLoginFailed)? diff --git a/crates/apub/src/activities/community/report.rs b/crates/apub/src/activities/community/report.rs index 4966add34..c33064459 100644 --- a/crates/apub/src/activities/community/report.rs +++ b/crates/apub/src/activities/community/report.rs @@ -70,7 +70,8 @@ impl Report { let object_creator = Person::read(&mut context.pool(), object_creator_id).await?; let object_creator_site: Option = Site::read_from_instance_id(&mut context.pool(), object_creator.instance_id) - .await? + .await + .ok() .map(Into::into); if let Some(inbox) = object_creator_site.map(|s| s.shared_inbox_or_inbox()) { inboxes.add_inbox(inbox); diff --git a/crates/apub/src/http/mod.rs b/crates/apub/src/http/mod.rs index d79cd3d55..8780f8789 100644 --- a/crates/apub/src/http/mod.rs +++ b/crates/apub/src/http/mod.rs @@ -18,7 +18,7 @@ use lemmy_db_schema::{ CommunityVisibility, }; use lemmy_db_views_actor::structs::CommunityFollowerView; -use lemmy_utils::error::{FederationError, LemmyErrorType, LemmyResult}; +use lemmy_utils::error::{FederationError, LemmyErrorExt, LemmyErrorType, LemmyResult}; use serde::{Deserialize, Serialize}; use std::{ops::Deref, time::Duration}; use tokio::time::timeout; @@ -46,7 +46,7 @@ pub async fn shared_inbox( // consider the activity broken and move on. timeout(INCOMING_ACTIVITY_TIMEOUT, receive_fut) .await - .map_err(|_| FederationError::InboxTimeout)? + .with_lemmy_type(FederationError::InboxTimeout.into())? } /// Convert the data to json and turn it into an HTTP Response with the correct ActivityPub @@ -109,7 +109,7 @@ pub(crate) async fn get_activity( .into(); let activity = SentActivity::read_from_apub_id(&mut context.pool(), &activity_id) .await - .map_err(|_| FederationError::CouldntFindActivity)?; + .with_lemmy_type(FederationError::CouldntFindActivity.into())?; let sensitive = activity.sensitive; if sensitive { diff --git a/crates/db_schema/src/impls/site.rs b/crates/db_schema/src/impls/site.rs index e993639fa..7ab13b8e2 100644 --- a/crates/db_schema/src/impls/site.rs +++ b/crates/db_schema/src/impls/site.rs @@ -10,7 +10,7 @@ use crate::{ }; use diesel::{dsl::insert_into, result::Error, ExpressionMethods, OptionalExtension, QueryDsl}; use diesel_async::RunQueryDsl; -use lemmy_utils::error::{LemmyErrorType, LemmyResult}; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; use url::Url; #[async_trait] @@ -65,13 +65,13 @@ impl Site { pub async fn read_from_instance_id( pool: &mut DbPool<'_>, _instance_id: InstanceId, - ) -> Result, Error> { + ) -> LemmyResult { let conn = &mut get_conn(pool).await?; site::table .filter(site::instance_id.eq(_instance_id)) .first(conn) .await - .optional() + .with_lemmy_type(LemmyErrorType::NotFound) } pub async fn read_from_apub_id( pool: &mut DbPool<'_>, diff --git a/crates/db_views_actor/src/community_follower_view.rs b/crates/db_views_actor/src/community_follower_view.rs index d3015c182..56f4cca93 100644 --- a/crates/db_views_actor/src/community_follower_view.rs +++ b/crates/db_views_actor/src/community_follower_view.rs @@ -21,7 +21,7 @@ use lemmy_db_schema::{ CommunityVisibility, SubscribedType, }; -use lemmy_utils::error::{LemmyErrorType, LemmyResult}; +use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult}; impl CommunityFollowerView { /// return a list of local community ids and remote inboxes that at least one user of the given @@ -30,7 +30,7 @@ impl CommunityFollowerView { pool: &mut DbPool<'_>, instance_id: InstanceId, published_since: chrono::DateTime, - ) -> Result, Error> { + ) -> LemmyResult> { let conn = &mut get_conn(pool).await?; // In most cases this will fetch the same url many times (the shared inbox url) // PG will only send a single copy to rust, but it has to scan through all follower rows (same @@ -51,6 +51,7 @@ impl CommunityFollowerView { .distinct() // only need each community_id, inbox combination once .load::<(CommunityId, DbUrl)>(conn) .await + .with_lemmy_type(LemmyErrorType::NotFound) } pub async fn get_community_follower_inboxes( pool: &mut DbPool<'_>, diff --git a/crates/federate/src/inboxes.rs b/crates/federate/src/inboxes.rs index 33c9c138c..ec96b1d6c 100644 --- a/crates/federate/src/inboxes.rs +++ b/crates/federate/src/inboxes.rs @@ -1,5 +1,4 @@ use crate::util::LEMMY_TEST_FAST_FEDERATION; -use anyhow::Result; use async_trait::async_trait; use chrono::{DateTime, TimeZone, Utc}; use lemmy_db_schema::{ @@ -8,6 +7,7 @@ use lemmy_db_schema::{ utils::{ActualDbPool, DbPool}, }; use lemmy_db_views_actor::structs::CommunityFollowerView; +use lemmy_utils::error::LemmyResult; use reqwest::Url; use std::{ collections::{HashMap, HashSet}, @@ -40,15 +40,12 @@ static FOLLOW_REMOVALS_RECHECK_DELAY: LazyLock = #[async_trait] pub trait DataSource: Send + Sync { - async fn read_site_from_instance_id( - &self, - instance_id: InstanceId, - ) -> Result, diesel::result::Error>; + async fn read_site_from_instance_id(&self, instance_id: InstanceId) -> LemmyResult; async fn get_instance_followed_community_inboxes( &self, instance_id: InstanceId, last_fetch: DateTime, - ) -> Result, diesel::result::Error>; + ) -> LemmyResult>; } pub struct DbDataSource { pool: ActualDbPool, @@ -62,10 +59,7 @@ impl DbDataSource { #[async_trait] impl DataSource for DbDataSource { - async fn read_site_from_instance_id( - &self, - instance_id: InstanceId, - ) -> Result, diesel::result::Error> { + async fn read_site_from_instance_id(&self, instance_id: InstanceId) -> LemmyResult { Site::read_from_instance_id(&mut DbPool::Pool(&self.pool), instance_id).await } @@ -73,7 +67,7 @@ impl DataSource for DbDataSource { &self, instance_id: InstanceId, last_fetch: DateTime, - ) -> Result, diesel::result::Error> { + ) -> LemmyResult> { CommunityFollowerView::get_instance_followed_community_inboxes( &mut DbPool::Pool(&self.pool), instance_id, @@ -128,7 +122,7 @@ impl CommunityInboxCollector { /// most often this will return 0 values (if instance doesn't care about the activity) /// or 1 value (the shared inbox) /// > 1 values only happens for non-lemmy software - pub async fn get_inbox_urls(&mut self, activity: &SentActivity) -> Result> { + pub async fn get_inbox_urls(&mut self, activity: &SentActivity) -> LemmyResult> { let mut inbox_urls: HashSet = HashSet::new(); if activity.send_all_instances { @@ -136,7 +130,8 @@ impl CommunityInboxCollector { self.site = self .data_source .read_site_from_instance_id(self.instance_id) - .await?; + .await + .ok(); self.site_loaded = true; } if let Some(site) = &self.site { @@ -170,7 +165,7 @@ impl CommunityInboxCollector { Ok(inbox_urls.into_iter().collect()) } - pub async fn update_communities(&mut self) -> Result<()> { + pub async fn update_communities(&mut self) -> LemmyResult<()> { if (Utc::now() - self.last_full_communities_fetch) > *FOLLOW_REMOVALS_RECHECK_DELAY { tracing::debug!("{}: fetching full list of communities", self.domain); // process removals every hour @@ -203,7 +198,7 @@ impl CommunityInboxCollector { &mut self, instance_id: InstanceId, last_fetch: DateTime, - ) -> Result<(HashMap>, DateTime)> { + ) -> LemmyResult<(HashMap>, DateTime)> { // update to time before fetch to ensure overlap. subtract some time to ensure overlap even if // published date is not exact let new_last_fetch = Utc::now() - *FOLLOW_ADDITIONS_RECHECK_DELAY / 2; @@ -238,12 +233,12 @@ mod tests { DataSource {} #[async_trait] impl DataSource for DataSource { - async fn read_site_from_instance_id(&self, instance_id: InstanceId) -> Result, diesel::result::Error>; + async fn read_site_from_instance_id(&self, instance_id: InstanceId) -> LemmyResult; async fn get_instance_followed_community_inboxes( &self, instance_id: InstanceId, last_fetch: DateTime, - ) -> Result, diesel::result::Error>; + ) -> LemmyResult>; } } @@ -301,7 +296,7 @@ mod tests { collector .data_source .expect_read_site_from_instance_id() - .return_once(move |_| Ok(Some(site))); + .return_once(move |_| Ok(site)); let activity = SentActivity { id: ActivityId(1), @@ -335,14 +330,8 @@ mod tests { .expect_get_instance_followed_community_inboxes() .return_once(move |_, _| { Ok(vec![ - ( - community_id, - Url::parse(url1).map_err(|_| diesel::NotFound)?.into(), - ), - ( - community_id, - Url::parse(url2).map_err(|_| diesel::NotFound)?.into(), - ), + (community_id, Url::parse(url1)?.into()), + (community_id, Url::parse(url2)?.into()), ]) }); @@ -430,20 +419,13 @@ mod tests { collector .data_source .expect_read_site_from_instance_id() - .return_once(move |_| Ok(Some(site))); + .return_once(move |_| Ok(site)); let subdomain_inbox = "https://follower.example.com/inbox"; collector .data_source .expect_get_instance_followed_community_inboxes() - .return_once(move |_, _| { - Ok(vec![( - community_id, - Url::parse(subdomain_inbox) - .map_err(|_| diesel::NotFound)? - .into(), - )]) - }); + .return_once(move |_, _| Ok(vec![(community_id, Url::parse(subdomain_inbox)?.into())])); collector.update_communities().await?; let user1_inbox = Url::parse("https://example.com/user1/inbox")?; @@ -496,26 +478,11 @@ mod tests { .returning(move |_, last_fetch| { if last_fetch == Utc.timestamp_nanos(0) { Ok(vec![ - ( - community_id1, - Url::parse(user1_inbox_str) - .map_err(|_| diesel::NotFound)? - .into(), - ), - ( - community_id2, - Url::parse(user2_inbox_str) - .map_err(|_| diesel::NotFound)? - .into(), - ), + (community_id1, Url::parse(user1_inbox_str)?.into()), + (community_id2, Url::parse(user2_inbox_str)?.into()), ]) } else { - Ok(vec![( - community_id3, - Url::parse(user3_inbox_str) - .map_err(|_| diesel::NotFound)? - .into(), - )]) + Ok(vec![(community_id3, Url::parse(user3_inbox_str)?.into())]) } }); @@ -569,7 +536,7 @@ mod tests { collector .data_source .expect_read_site_from_instance_id() - .return_once(move |_| Ok(Some(site))); + .return_once(move |_| Ok(site)); collector .data_source diff --git a/crates/federate/src/worker.rs b/crates/federate/src/worker.rs index e57cedbd0..047c5a5d6 100644 --- a/crates/federate/src/worker.rs +++ b/crates/federate/src/worker.rs @@ -24,6 +24,7 @@ use lemmy_db_schema::{ }, utils::{ActualDbPool, DbPool}, }; +use lemmy_utils::error::LemmyResult; use std::{collections::BinaryHeap, ops::Add, time::Duration}; use tokio::{ sync::mpsc::{self, UnboundedSender}, @@ -86,7 +87,7 @@ impl InstanceWorker { federation_worker_config: FederationWorkerConfig, stop: CancellationToken, stats_sender: UnboundedSender, - ) -> Result<(), anyhow::Error> { + ) -> LemmyResult<()> { let pool = config.to_request_data().inner_pool().clone(); let state = FederationQueueState::load(&mut DbPool::Pool(&pool), instance.id).await?; let (report_send_result, receive_send_result) = @@ -116,7 +117,7 @@ impl InstanceWorker { /// loop fetch new activities from db and send them to the inboxes of the given instances /// this worker only returns if (a) there is an internal error or (b) the cancellation token is /// cancelled (graceful exit) - async fn loop_until_stopped(&mut self) -> Result<()> { + async fn loop_until_stopped(&mut self) -> LemmyResult<()> { self.initial_fail_sleep().await?; let (mut last_sent_id, mut newest_id) = self.get_latest_ids().await?; @@ -149,12 +150,15 @@ impl InstanceWorker { }); // compare to next id based on incrementing if expected_next_id != Some(next_id_to_send.0) { - anyhow::bail!( - "{}: next id to send is not as expected: {:?} != {:?}", - self.instance.domain, - expected_next_id, - next_id_to_send - ) + return Err( + anyhow::anyhow!( + "{}: next id to send is not as expected: {:?} != {:?}", + self.instance.domain, + expected_next_id, + next_id_to_send + ) + .into(), + ); } } @@ -341,7 +345,7 @@ impl InstanceWorker { /// we collect the relevant inboxes in the main instance worker task, and only spawn the send task /// if we have inboxes to send to this limits CPU usage and reduces overhead for the (many) /// cases where we don't have any inboxes - async fn spawn_send_if_needed(&mut self, activity_id: ActivityId) -> Result<()> { + async fn spawn_send_if_needed(&mut self, activity_id: ActivityId) -> LemmyResult<()> { let Some(ele) = get_activity_cached(&mut self.pool(), activity_id) .await .context("failed reading activity from db")? @@ -357,11 +361,7 @@ impl InstanceWorker { return Ok(()); }; let activity = &ele.0; - let inbox_urls = self - .inbox_collector - .get_inbox_urls(activity) - .await - .context("failed figuring out inbox urls")?; + let inbox_urls = self.inbox_collector.get_inbox_urls(activity).await?; if inbox_urls.is_empty() { // this is the case when the activity is not relevant to this receiving instance (e.g. no user // subscribed to the relevant community) diff --git a/crates/utils/src/error.rs b/crates/utils/src/error.rs index af3b1089e..906a9006d 100644 --- a/crates/utils/src/error.rs +++ b/crates/utils/src/error.rs @@ -278,6 +278,12 @@ cfg_if! { } } + impl From for LemmyErrorType { + fn from(error: FederationError) -> Self { + LemmyErrorType::FederationError { error: Some(error) } + } + } + pub trait LemmyErrorExt> { fn with_lemmy_type(self, error_type: LemmyErrorType) -> LemmyResult; } diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index 17ba27696..bc4155e6b 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -329,7 +329,7 @@ pub fn build_url_str_without_scheme(url_str: &str) -> LemmyResult { // Set the scheme to http, then remove the http:// part url .set_scheme("http") - .map_err(|_| LemmyErrorType::InvalidUrl)?; + .map_err(|_e| LemmyErrorType::InvalidUrl)?; let mut out = url .to_string() diff --git a/src/lib.rs b/src/lib.rs index 7ce4e5503..319efd224 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -178,7 +178,7 @@ pub async fn start_lemmy_server(args: CmdArgs) -> LemmyResult<()> { .set(Box::new(move |d, c| { Box::pin(match_outgoing_activities(d, c)) })) - .map_err(|_| LemmyErrorType::Unknown("couldnt set function pointer".into()))?; + .map_err(|_e| LemmyErrorType::Unknown("couldnt set function pointer".into()))?; let request_data = federation_config.to_request_data(); let outgoing_activities_task = tokio::task::spawn(handle_outgoing_activities( diff --git a/src/main.rs b/src/main.rs index 6f0497dda..73cd0c1a3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -17,7 +17,7 @@ pub async fn main() -> LemmyResult<()> { rustls::crypto::ring::default_provider() .install_default() - .map_err(|_| LemmyErrorType::Unknown("Failed to install rustls crypto provider".into()))?; + .map_err(|_e| LemmyErrorType::Unknown("Failed to install rustls crypto provider".into()))?; start_lemmy_server(args).await?; Ok(())