From bcd628e26fe67454164d8565a89032e5c14c385f Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Mon, 21 Oct 2024 22:25:06 +0200 Subject: [PATCH] Inline `NewCategory::create_or_update()` The function is only used in our test suite, the update behavior isn't actually needed there and the return value is also not used anywhere. This makes it possible to significantly simplify the function to the point where there is no point anymore to keep the function. Inlining it then also allows us to merge a couple of calls into single queries that insert multiple rows at once. --- src/models/category.rs | 12 ---------- src/tests/krate/publish/categories.rs | 7 ++++-- src/tests/routes/categories/get.rs | 31 ++++++++++++++++++------- src/tests/routes/categories/list.rs | 15 ++++++++---- src/tests/routes/category_slugs/list.rs | 15 ++++++++---- src/tests/routes/crates/list.rs | 19 +++++++++------ src/tests/routes/summary.rs | 8 ++++--- 7 files changed, 64 insertions(+), 43 deletions(-) diff --git a/src/models/category.rs b/src/models/category.rs index 7cc1c418d87..bc146cdde43 100644 --- a/src/models/category.rs +++ b/src/models/category.rs @@ -131,18 +131,6 @@ pub struct NewCategory<'a> { pub description: &'a str, } -impl<'a> NewCategory<'a> { - /// Inserts the category into the database, or updates an existing one. - pub fn create_or_update(&self, conn: &mut impl Conn) -> QueryResult { - insert_into(categories::table) - .values(self) - .on_conflict(categories::slug) - .do_update() - .set(self) - .get_result(conn) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/tests/krate/publish/categories.rs b/src/tests/krate/publish/categories.rs index 4acc4e8dc75..7df137dc603 100644 --- a/src/tests/krate/publish/categories.rs +++ b/src/tests/krate/publish/categories.rs @@ -1,6 +1,8 @@ use crate::tests::builders::PublishBuilder; use crate::tests::new_category; use crate::tests::util::{RequestHelper, TestApp}; +use crates_io_database::schema::categories; +use diesel::{insert_into, RunQueryDsl}; use googletest::prelude::*; use http::StatusCode; use insta::{assert_json_snapshot, assert_snapshot}; @@ -10,8 +12,9 @@ async fn good_categories() { let (app, _, _, token) = TestApp::full().with_token(); let mut conn = app.db_conn(); - new_category("Category 1", "cat1", "Category 1 crates") - .create_or_update(&mut conn) + insert_into(categories::table) + .values(new_category("Category 1", "cat1", "Category 1 crates")) + .execute(&mut conn) .unwrap(); let crate_to_publish = PublishBuilder::new("foo_good_cat", "1.0.0").category("cat1"); diff --git a/src/tests/routes/categories/get.rs b/src/tests/routes/categories/get.rs index eea6128266c..8968aa1f961 100644 --- a/src/tests/routes/categories/get.rs +++ b/src/tests/routes/categories/get.rs @@ -2,6 +2,8 @@ use crate::models::Category; use crate::tests::builders::CrateBuilder; use crate::tests::new_category; use crate::tests::util::{MockAnonymousUser, RequestHelper, TestApp}; +use crates_io_database::schema::categories; +use diesel::{insert_into, RunQueryDsl}; use insta::assert_json_snapshot; use serde_json::Value; @@ -16,10 +18,14 @@ async fn show() { anon.get(url).await.assert_not_found(); // Create a category and a subcategory - assert_ok!(new_category("Foo Bar", "foo-bar", "Foo Bar crates").create_or_update(&mut conn)); - assert_ok!( - new_category("Foo Bar::Baz", "foo-bar::baz", "Baz crates").create_or_update(&mut conn) - ); + let cats = vec![ + new_category("Foo Bar", "foo-bar", "Foo Bar crates"), + new_category("Foo Bar::Baz", "foo-bar::baz", "Baz crates"), + ]; + + assert_ok!(insert_into(categories::table) + .values(cats) + .execute(&mut conn)); // The category and its subcategories should be in the json let json: Value = anon.get(url).await.good(); @@ -41,10 +47,14 @@ async fn update_crate() { let mut conn = app.db_conn(); let user = user.as_model(); - assert_ok!(new_category("cat1", "cat1", "Category 1 crates").create_or_update(&mut conn)); - assert_ok!( - new_category("Category 2", "category-2", "Category 2 crates").create_or_update(&mut conn) - ); + let cats = vec![ + new_category("cat1", "cat1", "Category 1 crates"), + new_category("Category 2", "category-2", "Category 2 crates"), + ]; + + assert_ok!(insert_into(categories::table) + .values(cats) + .execute(&mut conn)); let krate = CrateBuilder::new("foo_crate", user.id).expect_build(&mut conn); @@ -97,7 +107,10 @@ async fn update_crate() { assert_eq!(count(&anon, "category-2").await, 0); // Add a category and its subcategory - assert_ok!(new_category("cat1::bar", "cat1::bar", "bar crates").create_or_update(&mut conn)); + assert_ok!(insert_into(categories::table) + .values(new_category("cat1::bar", "cat1::bar", "bar crates")) + .execute(&mut conn)); + Category::update_crate(&mut conn, &krate, &["cat1", "cat1::bar"]).unwrap(); assert_eq!(count(&anon, "cat1").await, 1); diff --git a/src/tests/routes/categories/list.rs b/src/tests/routes/categories/list.rs index e1263ad2f15..eec4ba33fb2 100644 --- a/src/tests/routes/categories/list.rs +++ b/src/tests/routes/categories/list.rs @@ -1,5 +1,7 @@ use crate::tests::new_category; use crate::tests::util::{RequestHelper, TestApp}; +use crates_io_database::schema::categories; +use diesel::{insert_into, RunQueryDsl}; use insta::assert_json_snapshot; use serde_json::Value; @@ -13,11 +15,14 @@ async fn index() { assert_json_snapshot!(json); // Create a category and a subcategory - new_category("foo", "foo", "Foo crates") - .create_or_update(&mut conn) - .unwrap(); - new_category("foo::bar", "foo::bar", "Bar crates") - .create_or_update(&mut conn) + let cats = vec![ + new_category("foo", "foo", "Foo crates"), + new_category("foo::bar", "foo::bar", "Bar crates"), + ]; + + insert_into(categories::table) + .values(cats) + .execute(&mut conn) .unwrap(); // Only the top-level categories should be on the page diff --git a/src/tests/routes/category_slugs/list.rs b/src/tests/routes/category_slugs/list.rs index 9b5c5121b64..34e140135b2 100644 --- a/src/tests/routes/category_slugs/list.rs +++ b/src/tests/routes/category_slugs/list.rs @@ -1,5 +1,7 @@ use crate::tests::new_category; use crate::tests::util::{RequestHelper, TestApp}; +use crates_io_database::schema::categories; +use diesel::{insert_into, RunQueryDsl}; use insta::assert_json_snapshot; use serde_json::Value; @@ -8,11 +10,14 @@ async fn category_slugs_returns_all_slugs_in_alphabetical_order() { let (app, anon) = TestApp::init().empty(); let mut conn = app.db_conn(); - new_category("Foo", "foo", "For crates that foo") - .create_or_update(&mut conn) - .unwrap(); - new_category("Bar", "bar", "For crates that bar") - .create_or_update(&mut conn) + let cats = vec![ + new_category("Foo", "foo", "For crates that foo"), + new_category("Bar", "bar", "For crates that bar"), + ]; + + insert_into(categories::table) + .values(cats) + .execute(&mut conn) .unwrap(); let response: Value = anon.get("/api/v1/category_slugs").await.good(); diff --git a/src/tests/routes/crates/list.rs b/src/tests/routes/crates/list.rs index 2f017395af3..2fb7d0d8366 100644 --- a/src/tests/routes/crates/list.rs +++ b/src/tests/routes/crates/list.rs @@ -3,6 +3,7 @@ use crate::schema::crates; use crate::tests::builders::{CrateBuilder, VersionBuilder}; use crate::tests::util::{RequestHelper, TestApp}; use crate::tests::{new_category, new_user}; +use crates_io_database::schema::categories; use diesel::{dsl::*, prelude::*, update}; use googletest::prelude::*; use http::StatusCode; @@ -152,12 +153,14 @@ async fn index_queries() { assert_eq!(json.meta.total, 0); } - new_category("Category 1", "cat1", "Category 1 crates") - .create_or_update(&mut conn) - .unwrap(); + let cats = vec![ + new_category("Category 1", "cat1", "Category 1 crates"), + new_category("Category 1::Ba'r", "cat1::bar", "Ba'r crates"), + ]; - new_category("Category 1::Ba'r", "cat1::bar", "Ba'r crates") - .create_or_update(&mut conn) + insert_into(categories::table) + .values(cats) + .execute(&mut conn) .unwrap(); Category::update_crate(&mut conn, &krate, &["cat1"]).unwrap(); @@ -862,9 +865,11 @@ async fn test_default_sort_recent() { assert_eq!(json.crates[1].downloads, 20); } - new_category("Animal", "animal", "animal crates") - .create_or_update(&mut conn) + insert_into(categories::table) + .values(new_category("Animal", "animal", "animal crates")) + .execute(&mut conn) .unwrap(); + Category::update_crate(&mut conn, &green_crate, &["animal"]).unwrap(); Category::update_crate(&mut conn, &potato_crate, &["animal"]).unwrap(); diff --git a/src/tests/routes/summary.rs b/src/tests/routes/summary.rs index 49add01c86b..a0a59ae4e62 100644 --- a/src/tests/routes/summary.rs +++ b/src/tests/routes/summary.rs @@ -4,7 +4,8 @@ use crate::tests::new_category; use crate::tests::util::{RequestHelper, TestApp}; use crate::views::{EncodableCategory, EncodableCrate, EncodableKeyword}; use chrono::Utc; -use diesel::{update, Connection, ExpressionMethods, RunQueryDsl}; +use crates_io_database::schema::categories; +use diesel::{insert_into, update, Connection, ExpressionMethods, RunQueryDsl}; #[derive(Deserialize)] struct SummaryResponse { @@ -34,8 +35,9 @@ async fn summary_new_crates() { let now_ = Utc::now().naive_utc(); let now_plus_two = now_ + chrono::Duration::seconds(2); - new_category("Category 1", "cat1", "Category 1 crates") - .create_or_update(conn) + insert_into(categories::table) + .values(new_category("Category 1", "cat1", "Category 1 crates")) + .execute(conn) .unwrap(); CrateBuilder::new("some_downloads", user.id)