diff --git a/crates/lib/database_queries/src/queries/users/get_user_profile_by_username.rs b/crates/lib/database_queries/src/queries/users/get_user_profile_by_username.rs index 487b687e96..a3d78bb960 100644 --- a/crates/lib/database_queries/src/queries/users/get_user_profile_by_username.rs +++ b/crates/lib/database_queries/src/queries/users/get_user_profile_by_username.rs @@ -4,11 +4,12 @@ use container_common::anyhow_result::AnyhowResult; use crate::helpers::boolean_converters::i8_to_bool; use log::{info, warn, log}; use reusable_types::entity_visibility::EntityVisibility; -use sqlx::MySqlPool; +use sqlx::{MySqlPool, MySql}; use sqlx::error::DatabaseError; use sqlx::error::Error::Database; use sqlx::mysql::MySqlDatabaseError; use std::sync::Arc; +use sqlx::pool::PoolConnection; // TODO: Make non-`Serialize` and make the HTTP endpoints do the work #[derive(Serialize)] @@ -82,9 +83,18 @@ struct RawUserProfileRecord { maybe_mod_user_token: Option, } +//#[deprecated = "Use the PoolConnection method"] pub async fn get_user_profile_by_username( username: &str, mysql_pool: &MySqlPool +) -> AnyhowResult> { + let mut connection = mysql_pool.acquire().await?; + get_user_profile_by_username_from_connection(username, &mut connection).await +} + +pub async fn get_user_profile_by_username_from_connection<'a>( + username: &str, + connection: &'a mut PoolConnection ) -> AnyhowResult> { let maybe_profile_record = sqlx::query_as!( RawUserProfileRecord, @@ -119,7 +129,7 @@ WHERE "#, username, ) - .fetch_one(mysql_pool) + .fetch_one(connection) .await; let profile_record : RawUserProfileRecord = match maybe_profile_record { diff --git a/crates/lib/database_queries/src/queries/users/user_badges/list_user_badges.rs b/crates/lib/database_queries/src/queries/users/user_badges/list_user_badges.rs index 80769eb9a0..6b1250bc08 100644 --- a/crates/lib/database_queries/src/queries/users/user_badges/list_user_badges.rs +++ b/crates/lib/database_queries/src/queries/users/user_badges/list_user_badges.rs @@ -2,7 +2,8 @@ use anyhow::anyhow; use chrono::{DateTime, Utc}; use container_common::anyhow_result::AnyhowResult; use log::{warn, info}; -use sqlx::MySqlPool; +use sqlx::{MySql, MySqlPool}; +use sqlx::pool::PoolConnection; #[derive(Serialize)] pub struct UserBadgeForList { @@ -22,7 +23,7 @@ struct RawDbUserBadgeForList { } pub async fn list_user_badges( - mysql_pool: &MySqlPool, + mysql_connector: &mut PoolConnection, user_token: &str, ) -> AnyhowResult> { info!("listing user badges"); @@ -45,7 +46,7 @@ WHERE "#, user_token ) - .fetch_all(mysql_pool) + .fetch_all(mysql_connector) .await; let user_badges : Vec = match maybe_user_badges { diff --git a/crates/lib/http_server_common/src/util/timer.rs b/crates/lib/http_server_common/src/util/timer.rs index 0a75926f0c..63592da9dd 100644 --- a/crates/lib/http_server_common/src/util/timer.rs +++ b/crates/lib/http_server_common/src/util/timer.rs @@ -85,6 +85,8 @@ impl MultiBenchmarkingTimer { } } + /// Time an async function that does not require moving args. + /// An example of this is an async section that takes a `MySqlPool`, which is Sync/Send. pub async fn time_async_section( &mut self, section_name: &str, @@ -104,4 +106,29 @@ impl MultiBenchmarkingTimer { result } + + /// Time an async function, passing ownership of arguments before returning them. + /// This avoids the use of mutexes. + /// An example of this is an async section that takes an `&mut PoolConnection`, which is + /// not Sync/Send safe, so we instead send the entire `PoolConnection` until we're done. + pub async fn time_async_section_moving_args( + &mut self, + section_name: &str, + mut arg: Arg, + callback: F + ) -> (Arg, Ret) + where F: FnOnce(Arg) -> Fut, + Fut: Future, + { + let before = Instant::now(); + let (mut arg, result) = callback(arg).await; + let after = Instant::now(); + + self.recorded_sections.push(SectionTime { + section_name: section_name.to_string(), + duration: after.duration_since(before), + }); + + (arg, result) + } } diff --git a/crates/service/storyteller_web/src/http_server/endpoints/users/get_profile.rs b/crates/service/storyteller_web/src/http_server/endpoints/users/get_profile.rs index 18edb74fab..62ae371ac1 100644 --- a/crates/service/storyteller_web/src/http_server/endpoints/users/get_profile.rs +++ b/crates/service/storyteller_web/src/http_server/endpoints/users/get_profile.rs @@ -10,7 +10,7 @@ use anyhow::anyhow; use chrono::{DateTime, Utc}; use crate::http_server::web_utils::response_error_helpers::to_simple_json_error; use crate::server_state::ServerState; -use database_queries::queries::users::get_user_profile_by_username::{get_user_profile_by_username, UserProfileResult}; +use database_queries::queries::users::get_user_profile_by_username::{get_user_profile_by_username, UserProfileResult, get_user_profile_by_username_from_connection}; use database_queries::queries::users::user_badges::list_user_badges::UserBadgeForList; use database_queries::queries::users::user_badges::list_user_badges::list_user_badges; use http_server_common::request::get_request_header_optional::get_request_header_optional; @@ -103,11 +103,6 @@ impl fmt::Display for ProfileError { } } - -struct Timing { - -} - pub async fn get_profile_handler( http_request: HttpRequest, path: Path, @@ -115,18 +110,28 @@ pub async fn get_profile_handler( { let mut benchmark = MultiBenchmarkingTimer::new_started(); - let maybe_user_session = - benchmark.time_async_section("session checker", || async { - server_state + let mut pool_connection = server_state.mysql_pool.acquire() + .await + .map_err(|e| { + warn!("Could not acquire DB pool: {:?}", e); + ProfileError::ServerError + })?; + + let (mut pool_connection, maybe_user_session_fut) = + benchmark.time_async_section_moving_args("session checker", pool_connection, |mut pc| async { + let ret = server_state .session_checker - .maybe_get_user_session(&http_request, &server_state.mysql_pool) + .maybe_get_user_session_from_connection(&http_request, &mut pc) .await .map_err(|e| { warn!("Session checker error: {:?}", e); ProfileError::ServerError - }) + }); + (pc, ret) } - ).await?; + ).await; + + let maybe_user_session = maybe_user_session_fut?; let mut is_mod = false; @@ -134,9 +139,10 @@ pub async fn get_profile_handler( is_mod = user_session.can_ban_users; } - let maybe_user_profile = - benchmark.time_async_section("profile query", || async { - get_user_profile_by_username(&path.username, &server_state.mysql_pool).await + let (mut pool_connection, maybe_user_profile) = + benchmark.time_async_section_moving_args("profile query", pool_connection, |mut pc| async { + let ret = get_user_profile_by_username_from_connection(&path.username, &mut pc).await; + (pc, ret) }).await; let mut user_profile = match maybe_user_profile { @@ -161,16 +167,18 @@ pub async fn get_profile_handler( return Err(ProfileError::NotFound); } - let badges = - benchmark.time_async_section("badges query", || async { - list_user_badges(&server_state.mysql_pool, &user_profile.user_token) - .await - }) - .await - .unwrap_or_else(|err| { - warn!("Error querying badges: {:?}", err); - return Vec::new(); // NB: Fine if this fails. Not sure why it would. - }); + let (_pool_connection, maybe_badges) = + benchmark.time_async_section_moving_args("badges query", pool_connection, |mut pc| async { + let ret = list_user_badges(&mut pc, &user_profile.user_token) + .await; + (pc, ret) + }).await; + + let badges = maybe_badges + .unwrap_or_else(|err| { + warn!("Error querying badges: {:?}", err); + return Vec::new(); // NB: Fine if this fails. Not sure why it would. + }); let mut profile_for_response = UserProfileRecordForResponse { user_token: user_profile.user_token, diff --git a/crates/service/storyteller_web/src/http_server/web_utils/session_checker.rs b/crates/service/storyteller_web/src/http_server/web_utils/session_checker.rs index 19f64d8de1..c49e3cfbb6 100644 --- a/crates/service/storyteller_web/src/http_server/web_utils/session_checker.rs +++ b/crates/service/storyteller_web/src/http_server/web_utils/session_checker.rs @@ -5,8 +5,9 @@ use crate::http_server::web_utils::cookie_manager::CookieManager; use database_queries::column_types::record_visibility::RecordVisibility; use database_queries::helpers::boolean_converters::{nullable_i8_to_optional_bool, i8_to_bool}; use log::{info, warn}; -use sqlx::MySqlPool; +use sqlx::{MySqlPool, MySql}; use sqlx::error::Error::RowNotFound; +use sqlx::pool::PoolConnection; #[derive(Clone)] pub struct SessionChecker { @@ -164,7 +165,17 @@ AND deleted_at IS NULL } } - pub async fn maybe_get_user_session(&self, request: &HttpRequest, pool: &MySqlPool) + //#[deprecated = "Use the PoolConnection method"] + pub async fn maybe_get_user_session( + &self, + request: &HttpRequest, + pool: &MySqlPool) -> AnyhowResult> + { + let mut connection = pool.acquire().await?; + self.maybe_get_user_session_from_connection(request, &mut connection).await + } + + pub async fn maybe_get_user_session_from_connection(&self, request: &HttpRequest, pool: &mut PoolConnection) -> AnyhowResult> {