From fc425a9053f989aa66b9adf6cc66ecf796c89cc4 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Tue, 29 Oct 2024 01:22:15 +0100 Subject: [PATCH 1/3] models/category: Convert `subcategories()` and `parent_categories()` to async fns --- src/controllers/category.rs | 64 +++++++++++++++++++------------------ src/models/category.rs | 49 ++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 41 deletions(-) diff --git a/src/controllers/category.rs b/src/controllers/category.rs index 7945bfa77e5..5d9fa1d39b2 100644 --- a/src/controllers/category.rs +++ b/src/controllers/category.rs @@ -8,7 +8,7 @@ use crate::util::RequestUtils; use crate::views::{EncodableCategory, EncodableCategoryWithSubcategories}; use axum::extract::Path; use axum::Json; -use diesel::prelude::*; +use diesel::QueryDsl; use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; use http::request::Parts; use serde_json::Value; @@ -48,41 +48,43 @@ pub async fn index(app: AppState, req: Parts) -> AppResult> { /// Handles the `GET /categories/:category_id` route. pub async fn show(state: AppState, Path(slug): Path) -> AppResult> { - let conn = state.db_read().await?; - spawn_blocking(move || { - let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); + use diesel_async::RunQueryDsl; - let cat: Category = Category::by_slug(&slug).first(conn)?; - let subcats = cat - .subcategories(conn)? - .into_iter() - .map(Category::into) - .collect(); - let parents = cat - .parent_categories(conn)? - .into_iter() - .map(Category::into) - .collect(); - - let cat = EncodableCategory::from(cat); - let cat_with_subcats = EncodableCategoryWithSubcategories { - id: cat.id, - category: cat.category, - slug: cat.slug, - description: cat.description, - created_at: cat.created_at, - crates_cnt: cat.crates_cnt, - subcategories: subcats, - parent_categories: parents, - }; - - Ok(Json(json!({ "category": cat_with_subcats }))) - }) - .await + let mut conn = state.db_read().await?; + + let cat: Category = Category::by_slug(&slug).first(&mut conn).await?; + let subcats = cat + .subcategories(&mut conn) + .await? + .into_iter() + .map(Category::into) + .collect(); + let parents = cat + .parent_categories(&mut conn) + .await? + .into_iter() + .map(Category::into) + .collect(); + + let cat = EncodableCategory::from(cat); + let cat_with_subcats = EncodableCategoryWithSubcategories { + id: cat.id, + category: cat.category, + slug: cat.slug, + description: cat.description, + created_at: cat.created_at, + crates_cnt: cat.crates_cnt, + subcategories: subcats, + parent_categories: parents, + }; + + Ok(Json(json!({ "category": cat_with_subcats }))) } /// Handles the `GET /category_slugs` route. pub async fn slugs(state: AppState) -> AppResult> { + use diesel::RunQueryDsl; + let conn = state.db_read().await?; spawn_blocking(move || { let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); diff --git a/src/models/category.rs b/src/models/category.rs index 815c38ca3e7..f3d6e3a0e6f 100644 --- a/src/models/category.rs +++ b/src/models/category.rs @@ -1,5 +1,9 @@ use chrono::NaiveDateTime; -use diesel::{self, *}; +use diesel::{ + delete, dsl, insert_into, sql_query, ExpressionMethods, QueryDsl, QueryResult, + TextExpressionMethods, +}; +use diesel_async::AsyncPgConnection; use crate::models::Crate; use crate::schema::*; @@ -47,6 +51,7 @@ impl Category { crate_id: i32, slugs: &[&str], ) -> QueryResult> { + use diesel::RunQueryDsl; conn.transaction(|conn| { let categories: Vec = categories::table .filter(categories::slug.eq_any(slugs)) @@ -77,6 +82,7 @@ impl Category { } pub fn count_toplevel(conn: &mut impl Conn) -> QueryResult { + use diesel::RunQueryDsl; categories::table .filter(categories::category.not_like("%::%")) .count() @@ -90,6 +96,7 @@ impl Category { offset: i64, ) -> QueryResult> { use diesel::sql_types::Int8; + use diesel::RunQueryDsl; let sort_sql = match sort { "crates" => "ORDER BY crates_cnt DESC", @@ -104,24 +111,31 @@ impl Category { .load(conn) } - pub fn subcategories(&self, conn: &mut impl Conn) -> QueryResult> { + pub async fn subcategories(&self, conn: &mut AsyncPgConnection) -> QueryResult> { use diesel::sql_types::Text; + use diesel_async::RunQueryDsl; sql_query(include_str!("../subcategories.sql")) .bind::(&self.category) .load(conn) + .await } /// Gathers the parent categories from the top-level Category to the direct parent of this Category. /// Returns categories as a Vector in order of traversal, not including this Category. /// The intention is to be able to have slugs or parent categories arrayed in order, to /// offer the frontend, for examples, slugs to create links to each parent category in turn. - pub fn parent_categories(&self, conn: &mut impl Conn) -> QueryResult> { + pub async fn parent_categories( + &self, + conn: &mut AsyncPgConnection, + ) -> QueryResult> { use diesel::sql_types::Text; + use diesel_async::RunQueryDsl; sql_query(include_str!("../parent_categories.sql")) .bind::(&self.slug) .load(conn) + .await } } @@ -139,10 +153,13 @@ pub struct NewCategory<'a> { mod tests { use super::*; use crate::test_util::test_db_connection; + use crates_io_test_db::TestDatabase; + use diesel_async::AsyncConnection; #[test] fn category_toplevel_excludes_subcategories() { use self::categories; + use diesel::RunQueryDsl; let (_test_db, conn) = &mut test_db_connection(); insert_into(categories::table) .values(&vec![ @@ -174,6 +191,7 @@ mod tests { #[test] fn category_toplevel_orders_by_crates_cnt_when_sort_given() { use self::categories; + use diesel::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { ( @@ -209,6 +227,7 @@ mod tests { #[test] fn category_toplevel_applies_limit_and_offset() { use self::categories; + use diesel::RunQueryDsl; let (_test_db, conn) = &mut test_db_connection(); insert_into(categories::table) .values(&vec![ @@ -244,6 +263,7 @@ mod tests { #[test] fn category_toplevel_includes_subcategories_in_crate_cnt() { use self::categories; + use diesel::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { ( @@ -282,6 +302,7 @@ mod tests { #[test] fn category_toplevel_applies_limit_and_offset_after_grouping() { use self::categories; + use diesel::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { ( @@ -321,9 +342,10 @@ mod tests { assert_eq!(expected, cats); } - #[test] - fn category_parent_categories_includes_path_to_node_with_count() { + #[tokio::test] + async fn category_parent_categories_includes_path_to_node_with_count() { use self::categories; + use diesel_async::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { ( @@ -333,7 +355,9 @@ mod tests { ) }; - let (_test_db, conn) = &mut test_db_connection(); + let test_db = TestDatabase::new(); + let mut conn = AsyncPgConnection::establish(test_db.url()).await.unwrap(); + insert_into(categories::table) .values(&vec![ new_cat("Cat 1", "cat1", 1), @@ -345,12 +369,17 @@ mod tests { new_cat("Cat 2::Sub 2", "cat2::sub2", 5), new_cat("Cat 3", "cat3", 200), ]) - .execute(conn) + .execute(&mut conn) + .await + .unwrap(); + + let cat: Category = Category::by_slug("cat1::sub1") + .first(&mut conn) + .await .unwrap(); - let cat: Category = Category::by_slug("cat1::sub1").first(conn).unwrap(); - let subcats = cat.subcategories(conn).unwrap(); - let parents = cat.parent_categories(conn).unwrap(); + let subcats = cat.subcategories(&mut conn).await.unwrap(); + let parents = cat.parent_categories(&mut conn).await.unwrap(); assert_eq!(parents.len(), 1); assert_eq!(parents[0].slug, "cat1"); From b25201ecfc1ea9c38a1d1c44b370a07f1084bbd8 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Tue, 29 Oct 2024 01:26:25 +0100 Subject: [PATCH 2/3] controllers/category: Reduce `spawn_blocking()` usage --- src/controllers/category.rs | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/controllers/category.rs b/src/controllers/category.rs index 5d9fa1d39b2..8664f7121e0 100644 --- a/src/controllers/category.rs +++ b/src/controllers/category.rs @@ -83,25 +83,22 @@ pub async fn show(state: AppState, Path(slug): Path) -> AppResult AppResult> { - use diesel::RunQueryDsl; + use diesel_async::RunQueryDsl; - let conn = state.db_read().await?; - spawn_blocking(move || { - let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); + let mut conn = state.db_read().await?; - let slugs: Vec = categories::table - .select((categories::slug, categories::slug, categories::description)) - .order(categories::slug) - .load(conn)?; + let slugs: Vec = categories::table + .select((categories::slug, categories::slug, categories::description)) + .order(categories::slug) + .load(&mut conn) + .await?; - #[derive(Serialize, Queryable)] - struct Slug { - id: String, - slug: String, - description: String, - } + #[derive(Serialize, Queryable)] + struct Slug { + id: String, + slug: String, + description: String, + } - Ok(Json(json!({ "category_slugs": slugs }))) - }) - .await + Ok(Json(json!({ "category_slugs": slugs }))) } From b88c11075088fabcb38063f31b6e37ff560d2c4f Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Tue, 29 Oct 2024 01:33:23 +0100 Subject: [PATCH 3/3] models/category: Convert `toplevel()` and `count_toplevel()` to async fns --- src/controllers/category.rs | 41 ++++++---------- src/controllers/summary.rs | 14 +++--- src/models/category.rs | 98 ++++++++++++++++++++++--------------- 3 files changed, 83 insertions(+), 70 deletions(-) diff --git a/src/controllers/category.rs b/src/controllers/category.rs index 8664f7121e0..1099ef56aa4 100644 --- a/src/controllers/category.rs +++ b/src/controllers/category.rs @@ -2,14 +2,13 @@ use super::helpers::pagination::*; use crate::app::AppState; use crate::models::Category; use crate::schema::categories; -use crate::tasks::spawn_blocking; use crate::util::errors::AppResult; use crate::util::RequestUtils; use crate::views::{EncodableCategory, EncodableCategoryWithSubcategories}; use axum::extract::Path; use axum::Json; use diesel::QueryDsl; -use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; +use diesel_async::RunQueryDsl; use http::request::Parts; use serde_json::Value; @@ -20,36 +19,30 @@ pub async fn index(app: AppState, req: Parts) -> AppResult> { // to paginate this. let options = PaginationOptions::builder().gather(&req)?; - let conn = app.db_read().await?; - spawn_blocking(move || { - let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); + let mut conn = app.db_read().await?; - let query = req.query(); - let sort = query.get("sort").map_or("alpha", String::as_str); + let query = req.query(); + let sort = query.get("sort").map_or("alpha", String::as_str); - let offset = options.offset().unwrap_or_default(); + let offset = options.offset().unwrap_or_default(); - let categories = Category::toplevel(conn, sort, options.per_page, offset)?; - let categories = categories - .into_iter() - .map(Category::into) - .collect::>(); + let categories = Category::toplevel(&mut conn, sort, options.per_page, offset).await?; + let categories = categories + .into_iter() + .map(Category::into) + .collect::>(); - // Query for the total count of categories - let total = Category::count_toplevel(conn)?; + // Query for the total count of categories + let total = Category::count_toplevel(&mut conn).await?; - Ok(Json(json!({ - "categories": categories, - "meta": { "total": total }, - }))) - }) - .await + Ok(Json(json!({ + "categories": categories, + "meta": { "total": total }, + }))) } /// Handles the `GET /categories/:category_id` route. pub async fn show(state: AppState, Path(slug): Path) -> AppResult> { - use diesel_async::RunQueryDsl; - let mut conn = state.db_read().await?; let cat: Category = Category::by_slug(&slug).first(&mut conn).await?; @@ -83,8 +76,6 @@ pub async fn show(state: AppState, Path(slug): Path) -> AppResult AppResult> { - use diesel_async::RunQueryDsl; - let mut conn = state.db_read().await?; let slugs: Vec = categories::table diff --git a/src/controllers/summary.rs b/src/controllers/summary.rs index fdfc123beec..d92d520f30f 100644 --- a/src/controllers/summary.rs +++ b/src/controllers/summary.rs @@ -14,7 +14,14 @@ use serde_json::Value; /// Handles the `GET /summary` route. pub async fn summary(state: AppState) -> AppResult> { - let conn = state.db_read().await?; + let mut conn = state.db_read().await?; + + let popular_categories = Category::toplevel(&mut conn, "crates", 10, 0) + .await? + .into_iter() + .map(Category::into) + .collect::>(); + spawn_blocking(move || { let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); @@ -116,11 +123,6 @@ pub async fn summary(state: AppState) -> AppResult> { .map(Keyword::into) .collect::>(); - let popular_categories = Category::toplevel(conn, "crates", 10, 0)? - .into_iter() - .map(Category::into) - .collect::>(); - Ok(Json(json!({ "num_downloads": num_downloads, "num_crates": num_crates, diff --git a/src/models/category.rs b/src/models/category.rs index f3d6e3a0e6f..a23ddfd4c2c 100644 --- a/src/models/category.rs +++ b/src/models/category.rs @@ -81,22 +81,23 @@ impl Category { }) } - pub fn count_toplevel(conn: &mut impl Conn) -> QueryResult { - use diesel::RunQueryDsl; + pub async fn count_toplevel(conn: &mut AsyncPgConnection) -> QueryResult { + use diesel_async::RunQueryDsl; categories::table .filter(categories::category.not_like("%::%")) .count() .get_result(conn) + .await } - pub fn toplevel( - conn: &mut impl Conn, + pub async fn toplevel( + conn: &mut AsyncPgConnection, sort: &str, limit: i64, offset: i64, ) -> QueryResult> { use diesel::sql_types::Int8; - use diesel::RunQueryDsl; + use diesel_async::RunQueryDsl; let sort_sql = match sort { "crates" => "ORDER BY crates_cnt DESC", @@ -109,6 +110,7 @@ impl Category { .bind::(limit) .bind::(offset) .load(conn) + .await } pub async fn subcategories(&self, conn: &mut AsyncPgConnection) -> QueryResult> { @@ -152,15 +154,17 @@ pub struct NewCategory<'a> { #[cfg(test)] mod tests { use super::*; - use crate::test_util::test_db_connection; use crates_io_test_db::TestDatabase; use diesel_async::AsyncConnection; + use diesel_async::RunQueryDsl; - #[test] - fn category_toplevel_excludes_subcategories() { + #[tokio::test] + async fn category_toplevel_excludes_subcategories() { use self::categories; - use diesel::RunQueryDsl; - let (_test_db, conn) = &mut test_db_connection(); + + let test_db = TestDatabase::new(); + let mut conn = AsyncPgConnection::establish(test_db.url()).await.unwrap(); + insert_into(categories::table) .values(&vec![ ( @@ -176,10 +180,12 @@ mod tests { categories::slug.eq("cat1::sub"), ), ]) - .execute(conn) + .execute(&mut conn) + .await .unwrap(); - let cats = Category::toplevel(conn, "", 10, 0) + let cats = Category::toplevel(&mut conn, "", 10, 0) + .await .unwrap() .into_iter() .map(|c| c.category) @@ -188,10 +194,9 @@ mod tests { assert_eq!(expected, cats); } - #[test] - fn category_toplevel_orders_by_crates_cnt_when_sort_given() { + #[tokio::test] + async fn category_toplevel_orders_by_crates_cnt_when_sort_given() { use self::categories; - use diesel::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { ( @@ -201,17 +206,21 @@ mod tests { ) }; - let (_test_db, conn) = &mut test_db_connection(); + let test_db = TestDatabase::new(); + let mut conn = AsyncPgConnection::establish(test_db.url()).await.unwrap(); + insert_into(categories::table) .values(&vec![ new_cat("Cat 1", "cat1", 0), new_cat("Cat 2", "cat2", 2), new_cat("Cat 3", "cat3", 1), ]) - .execute(conn) + .execute(&mut conn) + .await .unwrap(); - let cats = Category::toplevel(conn, "crates", 10, 0) + let cats = Category::toplevel(&mut conn, "crates", 10, 0) + .await .unwrap() .into_iter() .map(|c| c.category) @@ -224,11 +233,13 @@ mod tests { assert_eq!(expected, cats); } - #[test] - fn category_toplevel_applies_limit_and_offset() { + #[tokio::test] + async fn category_toplevel_applies_limit_and_offset() { use self::categories; - use diesel::RunQueryDsl; - let (_test_db, conn) = &mut test_db_connection(); + + let test_db = TestDatabase::new(); + let mut conn = AsyncPgConnection::establish(test_db.url()).await.unwrap(); + insert_into(categories::table) .values(&vec![ ( @@ -240,10 +251,12 @@ mod tests { categories::slug.eq("cat2"), ), ]) - .execute(conn) + .execute(&mut conn) + .await .unwrap(); - let cats = Category::toplevel(conn, "", 1, 0) + let cats = Category::toplevel(&mut conn, "", 1, 0) + .await .unwrap() .into_iter() .map(|c| c.category) @@ -251,7 +264,8 @@ mod tests { let expected = vec!["Cat 1".to_string()]; assert_eq!(expected, cats); - let cats = Category::toplevel(conn, "", 1, 1) + let cats = Category::toplevel(&mut conn, "", 1, 1) + .await .unwrap() .into_iter() .map(|c| c.category) @@ -260,10 +274,9 @@ mod tests { assert_eq!(expected, cats); } - #[test] - fn category_toplevel_includes_subcategories_in_crate_cnt() { + #[tokio::test] + async fn category_toplevel_includes_subcategories_in_crate_cnt() { use self::categories; - use diesel::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { ( @@ -273,7 +286,9 @@ mod tests { ) }; - let (_test_db, conn) = &mut test_db_connection(); + let test_db = TestDatabase::new(); + let mut conn = AsyncPgConnection::establish(test_db.url()).await.unwrap(); + insert_into(categories::table) .values(&vec![ new_cat("Cat 1", "cat1", 1), @@ -283,10 +298,12 @@ mod tests { new_cat("Cat 2::Sub 2", "cat2::sub2", 5), new_cat("Cat 3", "cat3", 6), ]) - .execute(conn) + .execute(&mut conn) + .await .unwrap(); - let cats = Category::toplevel(conn, "crates", 10, 0) + let cats = Category::toplevel(&mut conn, "crates", 10, 0) + .await .unwrap() .into_iter() .map(|c| (c.category, c.crates_cnt)) @@ -299,10 +316,9 @@ mod tests { assert_eq!(expected, cats); } - #[test] - fn category_toplevel_applies_limit_and_offset_after_grouping() { + #[tokio::test] + async fn category_toplevel_applies_limit_and_offset_after_grouping() { use self::categories; - use diesel::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { ( @@ -312,7 +328,9 @@ mod tests { ) }; - let (_test_db, conn) = &mut test_db_connection(); + let test_db = TestDatabase::new(); + let mut conn = AsyncPgConnection::establish(test_db.url()).await.unwrap(); + insert_into(categories::table) .values(&vec![ new_cat("Cat 1", "cat1", 1), @@ -322,10 +340,12 @@ mod tests { new_cat("Cat 2::Sub 2", "cat2::sub2", 5), new_cat("Cat 3", "cat3", 6), ]) - .execute(conn) + .execute(&mut conn) + .await .unwrap(); - let cats = Category::toplevel(conn, "crates", 2, 0) + let cats = Category::toplevel(&mut conn, "crates", 2, 0) + .await .unwrap() .into_iter() .map(|c| (c.category, c.crates_cnt)) @@ -333,7 +353,8 @@ mod tests { let expected = vec![("Cat 2".to_string(), 12), ("Cat 3".to_string(), 6)]; assert_eq!(expected, cats); - let cats = Category::toplevel(conn, "crates", 2, 1) + let cats = Category::toplevel(&mut conn, "crates", 2, 1) + .await .unwrap() .into_iter() .map(|c| (c.category, c.crates_cnt)) @@ -345,7 +366,6 @@ mod tests { #[tokio::test] async fn category_parent_categories_includes_path_to_node_with_count() { use self::categories; - use diesel_async::RunQueryDsl; let new_cat = |category, slug, crates_cnt| { (