From 130b80324091efd55f70bcfc61438fccec983b3f Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Mon, 25 Mar 2024 12:19:16 +0100 Subject: [PATCH] avoid duplicate writing of new user languages --- .../src/local_user/generate_totp_secret.rs | 5 +- crates/api/src/local_user/update_totp.rs | 5 +- crates/api_common/src/claims.rs | 4 +- crates/api_crud/src/user/create.rs | 7 +- crates/apub/src/api/user_settings_backup.rs | 2 +- crates/db_schema/src/impls/actor_language.rs | 8 +- crates/db_schema/src/impls/local_user.rs | 93 ++++++++++--------- .../src/impls/password_reset_request.rs | 4 +- crates/db_views/src/comment_report_view.rs | 4 +- crates/db_views/src/comment_view.rs | 2 +- crates/db_views/src/post_report_view.rs | 4 +- crates/db_views/src/post_view.rs | 3 +- .../src/registration_application_view.rs | 6 +- crates/db_views_actor/src/community_view.rs | 4 +- crates/db_views_actor/src/person_view.rs | 4 +- src/code_migrations.rs | 2 +- src/session_middleware.rs | 4 +- 17 files changed, 86 insertions(+), 75 deletions(-) diff --git a/crates/api/src/local_user/generate_totp_secret.rs b/crates/api/src/local_user/generate_totp_secret.rs index a983beaaa..342a90a78 100644 --- a/crates/api/src/local_user/generate_totp_secret.rs +++ b/crates/api/src/local_user/generate_totp_secret.rs @@ -6,10 +6,7 @@ use lemmy_api_common::{ person::GenerateTotpSecretResponse, sensitive::Sensitive, }; -use lemmy_db_schema::{ - source::local_user::{LocalUser, LocalUserUpdateForm}, - traits::Crud, -}; +use lemmy_db_schema::source::local_user::{LocalUser, LocalUserUpdateForm}; use lemmy_db_views::structs::{LocalUserView, SiteView}; use lemmy_utils::error::{LemmyError, LemmyErrorType}; diff --git a/crates/api/src/local_user/update_totp.rs b/crates/api/src/local_user/update_totp.rs index 8f37213e2..c8ca9f64e 100644 --- a/crates/api/src/local_user/update_totp.rs +++ b/crates/api/src/local_user/update_totp.rs @@ -4,10 +4,7 @@ use lemmy_api_common::{ context::LemmyContext, person::{UpdateTotp, UpdateTotpResponse}, }; -use lemmy_db_schema::{ - source::local_user::{LocalUser, LocalUserUpdateForm}, - traits::Crud, -}; +use lemmy_db_schema::source::local_user::{LocalUser, LocalUserUpdateForm}; use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyError; diff --git a/crates/api_common/src/claims.rs b/crates/api_common/src/claims.rs index 0a96f7455..0e6968742 100644 --- a/crates/api_common/src/claims.rs +++ b/crates/api_common/src/claims.rs @@ -124,7 +124,9 @@ mod tests { .password_encrypted("123456".to_string()) .build(); - let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let inserted_local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let req = TestRequest::default().to_http_request(); let jwt = Claims::generate(inserted_local_user.id, req, &context) diff --git a/crates/api_crud/src/user/create.rs b/crates/api_crud/src/user/create.rs index 1c9398331..d24a287db 100644 --- a/crates/api_crud/src/user/create.rs +++ b/crates/api_crud/src/user/create.rs @@ -19,7 +19,6 @@ use lemmy_api_common::{ use lemmy_db_schema::{ aggregates::structs::PersonAggregates, source::{ - actor_language::LocalUserLanguage, captcha_answer::{CaptchaAnswer, CheckCaptchaAnswer}, language::Language, local_user::{LocalUser, LocalUserInsertForm}, @@ -155,8 +154,6 @@ pub async fn register( .admin(Some(!local_site.site_setup)) .build(); - let inserted_local_user = LocalUser::create(&mut context.pool(), &local_user_form).await?; - let all_languages = Language::read_all(&mut context.pool()).await?; // use hashset to avoid duplicates let mut language_ids = HashSet::new(); @@ -166,7 +163,9 @@ pub async fn register( } } let language_ids = language_ids.into_iter().collect(); - LocalUserLanguage::update(&mut context.pool(), language_ids, inserted_local_user.id).await?; + + let inserted_local_user = + LocalUser::create(&mut context.pool(), &local_user_form, language_ids).await?; if local_site.site_setup && require_registration_application { // Create the registration application diff --git a/crates/apub/src/api/user_settings_backup.rs b/crates/apub/src/api/user_settings_backup.rs index a2e6c55ac..d46e415b0 100644 --- a/crates/apub/src/api/user_settings_backup.rs +++ b/crates/apub/src/api/user_settings_backup.rs @@ -361,7 +361,7 @@ mod tests { .person_id(person.id) .password_encrypted("pass".to_string()) .build(); - let local_user = LocalUser::create(&mut context.pool(), &user_form).await?; + let local_user = LocalUser::create(&mut context.pool(), &user_form, vec![]).await?; Ok(LocalUserView::read(&mut context.pool(), local_user.id).await?) } diff --git a/crates/db_schema/src/impls/actor_language.rs b/crates/db_schema/src/impls/actor_language.rs index 6f317bbbc..29d99b2d8 100644 --- a/crates/db_schema/src/impls/actor_language.rs +++ b/crates/db_schema/src/impls/actor_language.rs @@ -535,7 +535,9 @@ mod tests { .password_encrypted("my_pw".to_string()) .build(); - let local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let local_user_langs1 = LocalUserLanguage::read(pool, local_user.id).await.unwrap(); // new user should be initialized with all languages @@ -648,7 +650,9 @@ mod tests { .person_id(person.id) .password_encrypted("my_pw".to_string()) .build(); - let local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); LocalUserLanguage::update(pool, test_langs2, local_user.id) .await .unwrap(); diff --git a/crates/db_schema/src/impls/local_user.rs b/crates/db_schema/src/impls/local_user.rs index 944557be5..d253afd8d 100644 --- a/crates/db_schema/src/impls/local_user.rs +++ b/crates/db_schema/src/impls/local_user.rs @@ -1,12 +1,11 @@ use crate::{ - newtypes::{DbUrl, LocalUserId, PersonId}, + newtypes::{DbUrl, LanguageId, LocalUserId, PersonId}, schema::{local_user, person, registration_application}, source::{ actor_language::LocalUserLanguage, local_user::{LocalUser, LocalUserInsertForm, LocalUserUpdateForm}, local_user_vote_display_mode::{LocalUserVoteDisplayMode, LocalUserVoteDisplayModeInsertForm}, }, - traits::Crud, utils::{ functions::{coalesce, lower}, get_conn, @@ -25,6 +24,52 @@ use diesel::{ use diesel_async::RunQueryDsl; impl LocalUser { + pub async fn create( + pool: &mut DbPool<'_>, + form: &LocalUserInsertForm, + languages: Vec, + ) -> Result { + let conn = &mut get_conn(pool).await?; + let mut form_with_encrypted_password = form.clone(); + let password_hash = + hash(&form.password_encrypted, DEFAULT_COST).expect("Couldn't hash password"); + form_with_encrypted_password.password_encrypted = password_hash; + + let local_user_ = insert_into(local_user::table) + .values(form_with_encrypted_password) + .get_result::(conn) + .await?; + + LocalUserLanguage::update(pool, languages, local_user_.id).await?; + + // Create their vote_display_modes + let vote_display_mode_form = LocalUserVoteDisplayModeInsertForm::builder() + .local_user_id(local_user_.id) + .build(); + LocalUserVoteDisplayMode::create(pool, &vote_display_mode_form).await?; + + Ok(local_user_) + } + + pub async fn update( + pool: &mut DbPool<'_>, + local_user_id: LocalUserId, + form: &LocalUserUpdateForm, + ) -> Result { + let conn = &mut get_conn(pool).await?; + diesel::update(local_user::table.find(local_user_id)) + .set(form) + .get_result::(conn) + .await + } + + pub async fn delete(pool: &mut DbPool<'_>, id: LocalUserId) -> Result { + let conn = &mut *get_conn(pool).await?; + diesel::delete(local_user::table.find(id)) + .execute(conn) + .await + } + pub async fn update_password( pool: &mut DbPool<'_>, local_user_id: LocalUserId, @@ -183,47 +228,3 @@ pub struct UserBackupLists { pub blocked_users: Vec, pub blocked_instances: Vec, } - -#[async_trait] -impl Crud for LocalUser { - type InsertForm = LocalUserInsertForm; - type UpdateForm = LocalUserUpdateForm; - type IdType = LocalUserId; - - async fn create(pool: &mut DbPool<'_>, form: &Self::InsertForm) -> Result { - let conn = &mut get_conn(pool).await?; - let mut form_with_encrypted_password = form.clone(); - let password_hash = - hash(&form.password_encrypted, DEFAULT_COST).expect("Couldn't hash password"); - form_with_encrypted_password.password_encrypted = password_hash; - - let local_user_ = insert_into(local_user::table) - .values(form_with_encrypted_password) - .get_result::(conn) - .await?; - - // TODO: this is necessary for tests, but causes unnecessary db writes in production as languages - // are set from accept-language header immediately after. would be good if final languages - // could be passed in directly. - LocalUserLanguage::update(pool, vec![], local_user_.id).await?; - - // Create their vote_display_modes - let vote_display_mode_form = LocalUserVoteDisplayModeInsertForm::builder() - .local_user_id(local_user_.id) - .build(); - LocalUserVoteDisplayMode::create(pool, &vote_display_mode_form).await?; - - Ok(local_user_) - } - async fn update( - pool: &mut DbPool<'_>, - local_user_id: LocalUserId, - form: &Self::UpdateForm, - ) -> Result { - let conn = &mut get_conn(pool).await?; - diesel::update(local_user::table.find(local_user_id)) - .set(form) - .get_result::(conn) - .await - } -} diff --git a/crates/db_schema/src/impls/password_reset_request.rs b/crates/db_schema/src/impls/password_reset_request.rs index 6fcf86014..3b910b1d3 100644 --- a/crates/db_schema/src/impls/password_reset_request.rs +++ b/crates/db_schema/src/impls/password_reset_request.rs @@ -121,7 +121,9 @@ mod tests { .password_encrypted("pass".to_string()) .build(); - let inserted_local_user = LocalUser::create(pool, &new_local_user).await.unwrap(); + let inserted_local_user = LocalUser::create(pool, &new_local_user, vec![]) + .await + .unwrap(); let token = "nope"; diff --git a/crates/db_views/src/comment_report_view.rs b/crates/db_views/src/comment_report_view.rs index 1aa3bb6e7..317ebf29a 100644 --- a/crates/db_views/src/comment_report_view.rs +++ b/crates/db_views/src/comment_report_view.rs @@ -308,7 +308,9 @@ mod tests { .person_id(inserted_timmy.id) .password_encrypted("123".to_string()) .build(); - let timmy_local_user = LocalUser::create(pool, &new_local_user).await.unwrap(); + let timmy_local_user = LocalUser::create(pool, &new_local_user, vec![]) + .await + .unwrap(); let timmy_view = LocalUserView { local_user: timmy_local_user, local_user_vote_display_mode: LocalUserVoteDisplayMode::default(), diff --git a/crates/db_views/src/comment_view.rs b/crates/db_views/src/comment_view.rs index 40065672c..0ba75c7b1 100644 --- a/crates/db_views/src/comment_view.rs +++ b/crates/db_views/src/comment_view.rs @@ -481,7 +481,7 @@ mod tests { .admin(Some(true)) .password_encrypted(String::new()) .build(); - let inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form) + let inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form, vec![]) .await .unwrap(); diff --git a/crates/db_views/src/post_report_view.rs b/crates/db_views/src/post_report_view.rs index 0f1f3639c..20795c8c0 100644 --- a/crates/db_views/src/post_report_view.rs +++ b/crates/db_views/src/post_report_view.rs @@ -330,7 +330,9 @@ mod tests { .person_id(inserted_timmy.id) .password_encrypted("123".to_string()) .build(); - let timmy_local_user = LocalUser::create(pool, &new_local_user).await.unwrap(); + let timmy_local_user = LocalUser::create(pool, &new_local_user, vec![]) + .await + .unwrap(); let timmy_view = LocalUserView { local_user: timmy_local_user, local_user_vote_display_mode: LocalUserVoteDisplayMode::default(), diff --git a/crates/db_views/src/post_view.rs b/crates/db_views/src/post_view.rs index b9522d47b..7292f7195 100644 --- a/crates/db_views/src/post_view.rs +++ b/crates/db_views/src/post_view.rs @@ -807,7 +807,7 @@ mod tests { admin: Some(true), ..LocalUserInsertForm::test_form(inserted_person.id) }; - let inserted_local_user = LocalUser::create(pool, &local_user_form).await?; + let inserted_local_user = LocalUser::create(pool, &local_user_form, vec![]).await?; let new_bot = PersonInsertForm { bot_account: Some(true), @@ -833,6 +833,7 @@ mod tests { let inserted_blocked_local_user = LocalUser::create( pool, &LocalUserInsertForm::test_form(inserted_blocked_person.id), + vec![], ) .await?; diff --git a/crates/db_views/src/registration_application_view.rs b/crates/db_views/src/registration_application_view.rs index a59158318..c2f2e3120 100644 --- a/crates/db_views/src/registration_application_view.rs +++ b/crates/db_views/src/registration_application_view.rs @@ -176,7 +176,7 @@ mod tests { .admin(Some(true)) .build(); - let _inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form) + let _inserted_timmy_local_user = LocalUser::create(pool, &timmy_local_user_form, vec![]) .await .unwrap(); @@ -193,7 +193,7 @@ mod tests { .password_encrypted("nada".to_string()) .build(); - let inserted_sara_local_user = LocalUser::create(pool, &sara_local_user_form) + let inserted_sara_local_user = LocalUser::create(pool, &sara_local_user_form, vec![]) .await .unwrap(); @@ -224,7 +224,7 @@ mod tests { .password_encrypted("nada".to_string()) .build(); - let inserted_jess_local_user = LocalUser::create(pool, &jess_local_user_form) + let inserted_jess_local_user = LocalUser::create(pool, &jess_local_user_form, vec![]) .await .unwrap(); diff --git a/crates/db_views_actor/src/community_view.rs b/crates/db_views_actor/src/community_view.rs index c1cb6eee1..98a146c15 100644 --- a/crates/db_views_actor/src/community_view.rs +++ b/crates/db_views_actor/src/community_view.rs @@ -296,7 +296,9 @@ mod tests { .person_id(inserted_person.id) .password_encrypted(String::new()) .build(); - let local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let new_community = CommunityInsertForm::builder() .name("test_community_3".to_string()) diff --git a/crates/db_views_actor/src/person_view.rs b/crates/db_views_actor/src/person_view.rs index aee56748e..1ee51a819 100644 --- a/crates/db_views_actor/src/person_view.rs +++ b/crates/db_views_actor/src/person_view.rs @@ -204,7 +204,7 @@ mod tests { .person_id(alice.id) .password_encrypted(String::new()) .build(); - let alice_local_user = LocalUser::create(pool, &alice_local_user_form).await?; + let alice_local_user = LocalUser::create(pool, &alice_local_user_form, vec![]).await?; let bob_form = PersonInsertForm::builder() .name("bob".to_string()) @@ -218,7 +218,7 @@ mod tests { .person_id(bob.id) .password_encrypted(String::new()) .build(); - let bob_local_user = LocalUser::create(pool, &bob_local_user_form).await?; + let bob_local_user = LocalUser::create(pool, &bob_local_user_form, vec![]).await?; Ok(Data { alice, diff --git a/src/code_migrations.rs b/src/code_migrations.rs index 8e17b8a8c..cee02075c 100644 --- a/src/code_migrations.rs +++ b/src/code_migrations.rs @@ -475,7 +475,7 @@ async fn initialize_local_site_2022_10_10( .email(setup.admin_email.clone()) .admin(Some(true)) .build(); - LocalUser::create(pool, &local_user_form).await?; + LocalUser::create(pool, &local_user_form, vec![]).await?; }; // Add an entry for the site table diff --git a/src/session_middleware.rs b/src/session_middleware.rs index 2c4e36913..474709dbe 100644 --- a/src/session_middleware.rs +++ b/src/session_middleware.rs @@ -155,7 +155,9 @@ mod tests { .password_encrypted("123456".to_string()) .build(); - let inserted_local_user = LocalUser::create(pool, &local_user_form).await.unwrap(); + let inserted_local_user = LocalUser::create(pool, &local_user_form, vec![]) + .await + .unwrap(); let req = TestRequest::default().to_http_request(); let jwt = Claims::generate(inserted_local_user.id, req, &context)