From 4e265dad33f7deb34c8204df8d6f2cb5b407c895 Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Mon, 26 Aug 2024 14:25:27 +0100 Subject: [PATCH 1/9] update: Initialized a new route to update user information; added new db schema to update user --- services/api/routes/src/update.rs | 82 +++++++++++++++++++++++++++++++ services/storage/src/users.rs | 39 +++++++++++++-- 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/services/api/routes/src/update.rs b/services/api/routes/src/update.rs index 8b13789..7e6fbe3 100644 --- a/services/api/routes/src/update.rs +++ b/services/api/routes/src/update.rs @@ -1 +1,83 @@ +/* +** Update route should handle updating the information of the existing user. +** A user is allowed to update information such as follows: +** Chosen notifier, set to null to disable notifications +** The user's email address +** User's telegram handle +** User's enabled notifications +** +** Validate user by exists by using only the ID. +** If the ID exists, then it can be validated. +** +** How can we ensure that the owner of the email | tg is making the update? +*/ +use crate::{errors::{custom_error, Error}, update, LOG_TARGET}; + +use common_macros::ensure; +use rocket::{http::Status, put, response::status, serde::json::Json, State}; +use rusqlite::Connection; +use serde::{Deserialize, Serialize}; +use storage::{users::User, DbConn}; +use types::{api::ErrorResponse, Notifier}; + +// If there is data that should not be updated, then pass current value. +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(crate = "rocket::serde")] +pub struct UpdateData { + // The ID of the user to update + pub id: u32, + // The email address to update to, + pub email: Option, + // The telegram handle to update to + pub tg_handle: Option, + // The desired notifier to use. + // If undefined, notifications will be turned off for user + // Pass current value if not to be updated + pub notifier: Option, + +} + +#[put("/update_user", data = "")] +pub async fn update_user( + conn: &State, + update_data: Json +) -> Result<(), status::Custom>> { + // validate the data passed + // Get data to update and serialize + // Update the data + log::info!(target: LOG_TARGET, "Update user request {:?}", update_data); + + // Get connection: + let conn = conn.lock().map_err(|err| { + log::error!(target: LOG_TARGET, "DB connection failed: {:?}", err); + custom_error(Status::InternalServerError, Error::DbConnectionFailed) + })?; + + // Ensure user exists + let db_user = verify_existing_id(&conn, update_data.id)?; + + let user = User { + email: update_data.email.clone(), + tg_handle: update_data.tg_handle.clone(), + id: update_data.id.clone(), + notifier: update_data.notifier.clone().unwrap(), + }; + // User::update_user(&conn, user); + Ok(()) +} + +fn verify_existing_id( + conn: &Connection, + user_id: u32, +) -> Result>> { + let maybe_user = User::query_by_id(&conn, user_id).map_err(|err| { + log::error!(target: LOG_TARGET, "Failed to searhc user by id: {:?}", err); + custom_error(Status::InternalServerError, Error::DbError) + })?; + + match maybe_user { + Some(user) => Ok(user), + None => Err(custom_error(Status::NotFound, Error::UserNotFound)) + } +} \ No newline at end of file diff --git a/services/storage/src/users.rs b/services/storage/src/users.rs index aa8a091..2712205 100644 --- a/services/storage/src/users.rs +++ b/services/storage/src/users.rs @@ -108,11 +108,7 @@ impl User { pub fn create_user(conn: &Connection, user: &User) -> Result<()> { let User { id, email, tg_handle, .. } = user; - let notifier = match user.notifier { - Notifier::Email => Some("email"), - Notifier::Telegram => Some("telegram"), - _ => None, - }; + let notifier = Self::notifier_to_text(&user.notifier); match notifier { Some(notifier) => { @@ -136,4 +132,37 @@ impl User { }; Ok(()) } + + pub fn update(conn: &Connection, user: &User) -> Result<()> { + let User { id, email, tg_handle, .. } = user; + let notifier = Self::notifier_to_text(&user.notifier); + + if notifier.is_some() { + conn.execute( + "UPDATE users SET + email = ?1, tg_handle = ?2, notifier = ?3 + WHERE id = ?4 + ", + params![email, tg_handle, notifier.unwrap(), id], + )?; + } else { + conn.execute( + "UPDATE users SET + email = ?1, tg_handle = ?2, notifier = NULL + WHERE id = ?3 + ", + params![email, tg_handle, id], + )?; + } + + Ok(()) + } + + fn notifier_to_text(notifier: &Notifier) -> Option { + match notifier { + Notifier::Email => Some(String::from("email")), + Notifier::Telegram => Some("telegram".to_string()), + _ => None, + } + } } From 6834010254f815917cbe68d4154d6228a7bfa130 Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Mon, 26 Aug 2024 14:26:43 +0100 Subject: [PATCH 2/9] update: hooked up updatuser route --- services/api/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/api/src/lib.rs b/services/api/src/lib.rs index 0b83b1b..d14aaeb 100644 --- a/services/api/src/lib.rs +++ b/services/api/src/lib.rs @@ -7,7 +7,7 @@ use rocket::{Build, Rocket}; use rocket_cors::CorsOptions; -use routes::{query::user, register::register_user}; +use routes::{query::user, register::register_user, update::update_user}; use storage_service::init_db; #[macro_use] @@ -20,7 +20,7 @@ pub async fn rocket() -> Rocket { rocket::build() .attach(CorsOptions::default().to_cors().unwrap()) .manage(connection) - .mount("/", routes![register_user, user]) + .mount("/", routes![register_user, user, update_user]) } // There should be three paths: one POST path to set the notification configuration, From bac581ba1fe2eaa763437e4d186f6ecac824f23a Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Mon, 26 Aug 2024 14:27:14 +0100 Subject: [PATCH 3/9] lint: cargo format fix-all formatting errors --- macros/src/lib.rs | 2 +- services/api/routes/src/update.rs | 92 ++++++++++++++++--------------- services/storage/src/users.rs | 4 +- 3 files changed, 50 insertions(+), 48 deletions(-) diff --git a/macros/src/lib.rs b/macros/src/lib.rs index e698890..8e807c0 100644 --- a/macros/src/lib.rs +++ b/macros/src/lib.rs @@ -2,7 +2,7 @@ macro_rules! ensure { ( $x:expr, $y:expr $(,)? ) => {{ if !$x { - return Err($y) + return Err($y); } }}; } diff --git a/services/api/routes/src/update.rs b/services/api/routes/src/update.rs index 7e6fbe3..1be69e8 100644 --- a/services/api/routes/src/update.rs +++ b/services/api/routes/src/update.rs @@ -12,7 +12,10 @@ ** How can we ensure that the owner of the email | tg is making the update? */ -use crate::{errors::{custom_error, Error}, update, LOG_TARGET}; +use crate::{ + errors::{custom_error, Error}, + update, LOG_TARGET, +}; use common_macros::ensure; use rocket::{http::Status, put, response::status, serde::json::Json, State}; @@ -25,59 +28,58 @@ use types::{api::ErrorResponse, Notifier}; #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] #[serde(crate = "rocket::serde")] pub struct UpdateData { - // The ID of the user to update - pub id: u32, - // The email address to update to, - pub email: Option, - // The telegram handle to update to - pub tg_handle: Option, - // The desired notifier to use. - // If undefined, notifications will be turned off for user - // Pass current value if not to be updated - pub notifier: Option, - + // The ID of the user to update + pub id: u32, + // The email address to update to, + pub email: Option, + // The telegram handle to update to + pub tg_handle: Option, + // The desired notifier to use. + // If undefined, notifications will be turned off for user + // Pass current value if not to be updated + pub notifier: Option, } #[put("/update_user", data = "")] pub async fn update_user( - conn: &State, - update_data: Json + conn: &State, + update_data: Json, ) -> Result<(), status::Custom>> { - // validate the data passed - // Get data to update and serialize - // Update the data - log::info!(target: LOG_TARGET, "Update user request {:?}", update_data); + // validate the data passed + // Get data to update and serialize + // Update the data + log::info!(target: LOG_TARGET, "Update user request {:?}", update_data); + + // Get connection: + let conn = conn.lock().map_err(|err| { + log::error!(target: LOG_TARGET, "DB connection failed: {:?}", err); + custom_error(Status::InternalServerError, Error::DbConnectionFailed) + })?; - // Get connection: - let conn = conn.lock().map_err(|err| { - log::error!(target: LOG_TARGET, "DB connection failed: {:?}", err); - custom_error(Status::InternalServerError, Error::DbConnectionFailed) - })?; + // Ensure user exists + let db_user = verify_existing_id(&conn, update_data.id)?; - // Ensure user exists - let db_user = verify_existing_id(&conn, update_data.id)?; - - let user = User { - email: update_data.email.clone(), - tg_handle: update_data.tg_handle.clone(), - id: update_data.id.clone(), - notifier: update_data.notifier.clone().unwrap(), - }; - // User::update_user(&conn, user); - Ok(()) + let user = User { + email: update_data.email.clone(), + tg_handle: update_data.tg_handle.clone(), + id: update_data.id.clone(), + notifier: update_data.notifier.clone().unwrap(), + }; + // User::update_user(&conn, user); + Ok(()) } fn verify_existing_id( - conn: &Connection, - user_id: u32, + conn: &Connection, + user_id: u32, ) -> Result>> { - let maybe_user = User::query_by_id(&conn, user_id).map_err(|err| { - log::error!(target: LOG_TARGET, "Failed to searhc user by id: {:?}", err); - custom_error(Status::InternalServerError, Error::DbError) - })?; + let maybe_user = User::query_by_id(&conn, user_id).map_err(|err| { + log::error!(target: LOG_TARGET, "Failed to searhc user by id: {:?}", err); + custom_error(Status::InternalServerError, Error::DbError) + })?; - match maybe_user { - Some(user) => Ok(user), - None => Err(custom_error(Status::NotFound, Error::UserNotFound)) - } -} \ No newline at end of file + match maybe_user { + Some(user) => Ok(user), + None => Err(custom_error(Status::NotFound, Error::UserNotFound)), + } +} diff --git a/services/storage/src/users.rs b/services/storage/src/users.rs index 2712205..cd33068 100644 --- a/services/storage/src/users.rs +++ b/services/storage/src/users.rs @@ -142,7 +142,7 @@ impl User { "UPDATE users SET email = ?1, tg_handle = ?2, notifier = ?3 WHERE id = ?4 - ", + ", params![email, tg_handle, notifier.unwrap(), id], )?; } else { @@ -150,7 +150,7 @@ impl User { "UPDATE users SET email = ?1, tg_handle = ?2, notifier = NULL WHERE id = ?3 - ", + ", params![email, tg_handle, id], )?; } From f30a89af8ee65340f7d8a6790e06e82c76e581ca Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Tue, 27 Aug 2024 11:54:06 +0100 Subject: [PATCH 4/9] update: hooked up API for update user to store users on DB --- services/api/routes/src/register.rs | 8 ------- services/api/routes/src/update.rs | 23 ++++++++++++------ services/storage/src/users.rs | 36 ++++++++++++----------------- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/services/api/routes/src/register.rs b/services/api/routes/src/register.rs index e0a6ccc..b9aecc8 100644 --- a/services/api/routes/src/register.rs +++ b/services/api/routes/src/register.rs @@ -79,14 +79,6 @@ fn ensure_unique_data( conn: &Connection, registration_data: &Json, ) -> Result<(), status::Custom>> { - let maybe_user = User::query_by_id(&conn, registration_data.id).map_err(|err| { - log::error!(target: LOG_TARGET, "Failed to search user by id: {:?}", err); - custom_error(Status::InternalServerError, Error::DbError) - })?; - - // Ensure that the id is unique. - ensure!(maybe_user.is_none(), custom_error(Status::Conflict, Error::UserExists)); - let error = custom_error(Status::Conflict, Error::NotifierNotUnique); if let Some(email) = registration_data.email.clone() { diff --git a/services/api/routes/src/update.rs b/services/api/routes/src/update.rs index 1be69e8..093f213 100644 --- a/services/api/routes/src/update.rs +++ b/services/api/routes/src/update.rs @@ -32,6 +32,7 @@ pub struct UpdateData { pub id: u32, // The email address to update to, pub email: Option, + #[serde(rename = "tgHandle")] // The telegram handle to update to pub tg_handle: Option, // The desired notifier to use. @@ -44,7 +45,7 @@ pub struct UpdateData { pub async fn update_user( conn: &State, update_data: Json, -) -> Result<(), status::Custom>> { +) -> Result, status::Custom>> { // validate the data passed // Get data to update and serialize // Update the data @@ -57,16 +58,24 @@ pub async fn update_user( })?; // Ensure user exists - let db_user = verify_existing_id(&conn, update_data.id)?; + let db_user = verify_existing_id(&conn, update_data.id.clone())?; let user = User { email: update_data.email.clone(), tg_handle: update_data.tg_handle.clone(), id: update_data.id.clone(), - notifier: update_data.notifier.clone().unwrap(), - }; - // User::update_user(&conn, user); - Ok(()) + notifier: if update_data.notifier.clone().is_some() { + update_data.notifier.clone().unwrap() + } else { + db_user.notifier + }, + }; + let result = User::update(&conn, &user); + + match result { + Ok(_) => Ok(status::Custom(Status::Ok, ())), + Err(_) => Err(custom_error(Status::InternalServerError, Error::DbError)), + } } fn verify_existing_id( @@ -74,7 +83,7 @@ fn verify_existing_id( user_id: u32, ) -> Result>> { let maybe_user = User::query_by_id(&conn, user_id).map_err(|err| { - log::error!(target: LOG_TARGET, "Failed to searhc user by id: {:?}", err); + log::error!(target: LOG_TARGET, "Failed to search user by id: {:?}", err); custom_error(Status::InternalServerError, Error::DbError) })?; diff --git a/services/storage/src/users.rs b/services/storage/src/users.rs index cd33068..880fbb5 100644 --- a/services/storage/src/users.rs +++ b/services/storage/src/users.rs @@ -1,4 +1,4 @@ -use rusqlite::{params, Connection, Result}; +use rusqlite::{params, Connection, Error, Result}; use serde::{Deserialize, Serialize}; use types::Notifier; @@ -114,48 +114,42 @@ impl User { Some(notifier) => { conn.execute( "INSERT INTO users - (id, email, tg_handle, notifier) - VALUES (?1, ?2, ?3, ?4) + (email, tg_handle, notifier) + VALUES (?1, ?2, ?3) ", - params![id, email, tg_handle, notifier], + params![email, tg_handle, notifier], )?; }, None => { conn.execute( "INSERT INTO users (email, tg_handle, notifier) - VALUES (?1, ?2, ?3, NULL) + VALUES (?1, ?2, NULL) ", - params![id, email, tg_handle], + params![email, tg_handle], )?; }, }; Ok(()) } - pub fn update(conn: &Connection, user: &User) -> Result<()> { + pub fn update(conn: &Connection, user: &User) -> Result { let User { id, email, tg_handle, .. } = user; let notifier = Self::notifier_to_text(&user.notifier); - if notifier.is_some() { + let result = if notifier.is_some() { conn.execute( - "UPDATE users SET - email = ?1, tg_handle = ?2, notifier = ?3 - WHERE id = ?4 - ", - params![email, tg_handle, notifier.unwrap(), id], - )?; + "UPDATE users SET email = ?1, tg_handle = ?2, notifier = ?3 WHERE id = ?4", + params![email, tg_handle, notifier, id], + ) } else { conn.execute( - "UPDATE users SET - email = ?1, tg_handle = ?2, notifier = NULL - WHERE id = ?3 - ", + "UPDATE users SET email = ?1, tg_handle = ?2, notifier = NULL WHERE id = ?3", params![email, tg_handle, id], - )?; - } + ) + }; - Ok(()) + result } fn notifier_to_text(notifier: &Notifier) -> Option { From a4dd9d2be95a1b0e70f024adb4ab94731811a433 Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Tue, 27 Aug 2024 13:45:42 +0100 Subject: [PATCH 5/9] restore logic to register users with an ID, no autogenerate --- services/api/routes/src/register.rs | 8 ++++++++ services/storage/src/users.rs | 8 ++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/services/api/routes/src/register.rs b/services/api/routes/src/register.rs index b9aecc8..e0a6ccc 100644 --- a/services/api/routes/src/register.rs +++ b/services/api/routes/src/register.rs @@ -79,6 +79,14 @@ fn ensure_unique_data( conn: &Connection, registration_data: &Json, ) -> Result<(), status::Custom>> { + let maybe_user = User::query_by_id(&conn, registration_data.id).map_err(|err| { + log::error!(target: LOG_TARGET, "Failed to search user by id: {:?}", err); + custom_error(Status::InternalServerError, Error::DbError) + })?; + + // Ensure that the id is unique. + ensure!(maybe_user.is_none(), custom_error(Status::Conflict, Error::UserExists)); + let error = custom_error(Status::Conflict, Error::NotifierNotUnique); if let Some(email) = registration_data.email.clone() { diff --git a/services/storage/src/users.rs b/services/storage/src/users.rs index 880fbb5..773c3ff 100644 --- a/services/storage/src/users.rs +++ b/services/storage/src/users.rs @@ -115,18 +115,18 @@ impl User { conn.execute( "INSERT INTO users (email, tg_handle, notifier) - VALUES (?1, ?2, ?3) + VALUES (?1, ?2, ?3, ?4) ", - params![email, tg_handle, notifier], + params![id, email, tg_handle, notifier], )?; }, None => { conn.execute( "INSERT INTO users (email, tg_handle, notifier) - VALUES (?1, ?2, NULL) + VALUES (?1, ?2, ?3, NULL) ", - params![email, tg_handle], + params![id, email, tg_handle], )?; }, }; From d842ef456fc5397f0559d1787b18109fa9f4b5b4 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 27 Aug 2024 20:16:46 +0200 Subject: [PATCH 6/9] update docs formatting --- services/api/routes/src/update.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/services/api/routes/src/update.rs b/services/api/routes/src/update.rs index 093f213..73d191d 100644 --- a/services/api/routes/src/update.rs +++ b/services/api/routes/src/update.rs @@ -1,16 +1,16 @@ -/* -** Update route should handle updating the information of the existing user. -** A user is allowed to update information such as follows: -** Chosen notifier, set to null to disable notifications -** The user's email address -** User's telegram handle -** User's enabled notifications -** -** Validate user by exists by using only the ID. -** If the ID exists, then it can be validated. -** -** How can we ensure that the owner of the email | tg is making the update? -*/ +//! ## Update Route +//! +//! Update route should handle updating the information of the existing user. +//! A user is allowed to update the following information: +//! - User's notifier (set to null to disable notifications) +//! - User's email address +//! - User's telegram handle +//! - User's enabled notifications +//! +//! We ensure the user exists before validating. +//! If the ID exists, then it can be validated. +//! +//! How can we ensure that the owner of the email | tg is making the update? use crate::{ errors::{custom_error, Error}, From 63123d9709f271876b6e1fb86e245fb4300af06e Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:31:02 +0100 Subject: [PATCH 7/9] add unit tests to update api --- services/api/routes/src/query.rs | 1 + services/api/routes/src/tests/mod.rs | 1 + services/api/routes/src/tests/update.rs | 87 +++++++++++++++++++++++++ services/api/routes/src/update.rs | 17 ++++- 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 services/api/routes/src/tests/update.rs diff --git a/services/api/routes/src/query.rs b/services/api/routes/src/query.rs index 3ba85eb..015f7a8 100644 --- a/services/api/routes/src/query.rs +++ b/services/api/routes/src/query.rs @@ -32,5 +32,6 @@ pub async fn user( custom_error(Status::InternalServerError, Error::FailedToSerialize) })?; + log::info!(target: LOG_TARGET, "Queried User: {:?}", serialized); Ok(status::Custom(Status::Ok, serialized)) } diff --git a/services/api/routes/src/tests/mod.rs b/services/api/routes/src/tests/mod.rs index da49b26..22ff8db 100644 --- a/services/api/routes/src/tests/mod.rs +++ b/services/api/routes/src/tests/mod.rs @@ -1,3 +1,4 @@ mod mock; mod query; mod register; +mod update; \ No newline at end of file diff --git a/services/api/routes/src/tests/update.rs b/services/api/routes/src/tests/update.rs new file mode 100644 index 0000000..cd280e5 --- /dev/null +++ b/services/api/routes/src/tests/update.rs @@ -0,0 +1,87 @@ +use rocket::{http::{ContentType, Status}, local::blocking::{Client, LocalResponse}, routes}; +use storage::{init_db, users::User}; +use types::Notifier; + +use crate::{query::user, register::{register_user, RegistrationData}, update::{update_user, UpdateData}, LOG_TARGET}; + +use super::mock::execute_with; + +pub const DB_PATH: &'static str = "update-tests.db"; + +#[test] +fn updating_users_works() { + execute_with(DB_PATH, || { + let conn = init_db(DB_PATH).unwrap(); + let rocket = rocket::build().manage(conn).mount("/", routes![register_user, user, update_user]); + let client = Client::tracked(rocket).expect("failed to create client"); + + let registration_data = RegistrationData { + id: 0, + notifier: Notifier::Telegram, + email: None, + tg_handle: Some("@dummy".to_string()), + enabled_notifications: vec![], + }; + + // Should register successfully + let response = register(&client, ®istration_data); + assert_eq!(response.status(), Status::Ok); + + let response = client.get("/user/0").dispatch(); + assert_eq!(parse_ok_response(response), User { + id: 0, + email: None, + tg_handle: Some("@dummy".to_string()), + notifier: Notifier::Telegram, + }); + + // Update to mismatched notifier should not work + let mut update_data = UpdateData { + id: 0, + email: None, + tg_handle: Some("@dummy".to_string()), + notifier: Some(Notifier::Email), + }; + let response = update(&client, &update_data); + assert_eq!(response.status(), Status::BadRequest); + + // Update email with notifier works + let update_data = UpdateData { + id: 0, + email: Some("dummy@mail.com".to_string()), + tg_handle: Some("@dummy".to_string()), + notifier: Some(Notifier::Email) + }; + let response = update(&client, &update_data); + assert_eq!(response.status(), Status::Ok); + + // Should return the updated user information + let response = client.get("/user/0").dispatch(); + assert_eq!(parse_ok_response(response), User { + id: 0, + email: Some("dummy@mail.com".to_string()), + tg_handle: Some("@dummy".to_string()), + notifier: Notifier::Email, + }); + }) +} + +fn register<'a>(client: &'a Client, data: &'a RegistrationData) -> LocalResponse<'a> { + client + .post("/register_user") + .header(ContentType::JSON) + .body(serde_json::to_string(&data).unwrap()) + .dispatch() +} + +fn update<'a>(client: &'a Client, data: &'a UpdateData) -> LocalResponse<'a> { + client.put("/update_user") + .header(ContentType::JSON) + .body(serde_json::to_string(&data).unwrap()) + .dispatch() +} + +fn parse_ok_response<'a>(response: LocalResponse<'a>) -> User { + let body = response.into_string().unwrap(); + serde_json::from_str(&body).expect("can't parse value") +} \ No newline at end of file diff --git a/services/api/routes/src/update.rs b/services/api/routes/src/update.rs index 73d191d..4e75bec 100644 --- a/services/api/routes/src/update.rs +++ b/services/api/routes/src/update.rs @@ -41,16 +41,31 @@ pub struct UpdateData { pub notifier: Option, } +impl UpdateData { + fn validate(&self) -> Result<(), Error> { + // Ensure the configured notifier is set. + match &self.notifier { + Some(Notifier::Email) if self.email.is_none() => Err(Error::NotifierEmpty), + Some(Notifier::Telegram) if self.tg_handle.is_none() => Err(Error::NotifierEmpty), + _ => Ok(()), + } + } +} + #[put("/update_user", data = "")] pub async fn update_user( conn: &State, update_data: Json, ) -> Result, status::Custom>> { - // validate the data passed // Get data to update and serialize // Update the data log::info!(target: LOG_TARGET, "Update user request {:?}", update_data); + // validate the data passed + update_data + .validate() + .map_err(|err| custom_error(Status::BadRequest, err))?; + // Get connection: let conn = conn.lock().map_err(|err| { log::error!(target: LOG_TARGET, "DB connection failed: {:?}", err); From d158e583530f3470eaa09ddeb5c7c56aaec453a6 Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:52:21 +0100 Subject: [PATCH 8/9] fix: organized code imports --- services/api/routes/src/tests/mock.rs | 17 +++++++++++++++++ services/api/routes/src/tests/query.rs | 13 +------------ services/api/routes/src/tests/register.rs | 13 +------------ services/api/routes/src/tests/update.rs | 7 +------ 4 files changed, 20 insertions(+), 30 deletions(-) diff --git a/services/api/routes/src/tests/mock.rs b/services/api/routes/src/tests/mock.rs index 75764a4..86f4428 100644 --- a/services/api/routes/src/tests/mock.rs +++ b/services/api/routes/src/tests/mock.rs @@ -1,3 +1,9 @@ +use rocket::local::blocking::LocalResponse; +use storage::users::User; +use types::api::ErrorResponse; + +use crate::errors::Error; + pub fn execute_with(db_path: &str, f: impl Fn() -> R) -> R { // Don't check the result since it will error if the db already doesn't exist which isn't an // issue. @@ -5,3 +11,14 @@ pub fn execute_with(db_path: &str, f: impl Fn() -> R) -> R { f() } + +pub fn parse_ok_response<'a>(response: LocalResponse<'a>) -> User { + let body = response.into_string().unwrap(); + serde_json::from_str(&body).expect("can't parse value") +} + +pub fn parse_err_response<'a>(response: LocalResponse<'a>) -> Error { + let body = response.into_string().unwrap(); + let error: ErrorResponse = serde_json::from_str(&body).unwrap(); + error.message.into() +} \ No newline at end of file diff --git a/services/api/routes/src/tests/query.rs b/services/api/routes/src/tests/query.rs index a2181f4..d9f36df 100644 --- a/services/api/routes/src/tests/query.rs +++ b/services/api/routes/src/tests/query.rs @@ -2,7 +2,7 @@ use crate::{ errors::Error, query::user, register::{register_user, RegistrationData}, - tests::mock::execute_with, + tests::mock::{execute_with, parse_err_response, parse_ok_response}, }; use rocket::{ http::{ContentType, Status}, @@ -62,14 +62,3 @@ fn register<'a>(client: &'a Client, data: &'a RegistrationData) -> LocalResponse .body(serde_json::to_string(&data).unwrap()) .dispatch() } - -fn parse_ok_response<'a>(response: LocalResponse<'a>) -> User { - let body = response.into_string().unwrap(); - serde_json::from_str(&body).expect("can't parse value") -} - -fn parse_err_response<'a>(response: LocalResponse<'a>) -> Error { - let body = response.into_string().unwrap(); - let error: ErrorResponse = from_str(&body).unwrap(); - error.message.into() -} diff --git a/services/api/routes/src/tests/register.rs b/services/api/routes/src/tests/register.rs index 866fff9..7040440 100644 --- a/services/api/routes/src/tests/register.rs +++ b/services/api/routes/src/tests/register.rs @@ -2,7 +2,7 @@ use crate::{ errors::Error, query::user, register::{register_user, RegistrationData}, - tests::mock::execute_with, + tests::mock::{execute_with, parse_err_response, parse_ok_response}, }; use rocket::{ http::{ContentType, Status}, @@ -103,14 +103,3 @@ fn register<'a>(client: &'a Client, data: &'a RegistrationData) -> LocalResponse .body(serde_json::to_string(&data).unwrap()) .dispatch() } - -fn parse_ok_response<'a>(response: LocalResponse<'a>) -> User { - let body = response.into_string().unwrap(); - serde_json::from_str(&body).expect("can't parse value") -} - -fn parse_err_response<'a>(response: LocalResponse<'a>) -> Error { - let body = response.into_string().unwrap(); - let error: ErrorResponse = from_str(&body).unwrap(); - error.message.into() -} diff --git a/services/api/routes/src/tests/update.rs b/services/api/routes/src/tests/update.rs index cd280e5..b0b3f97 100644 --- a/services/api/routes/src/tests/update.rs +++ b/services/api/routes/src/tests/update.rs @@ -2,7 +2,7 @@ use rocket::{http::{ContentType, Status}, local::blocking::{Client, LocalRespons use storage::{init_db, users::User}; use types::Notifier; -use crate::{query::user, register::{register_user, RegistrationData}, update::{update_user, UpdateData}, LOG_TARGET}; +use crate::{query::user, register::{register_user, RegistrationData}, tests::mock::parse_ok_response, update::{update_user, UpdateData}, LOG_TARGET}; use super::mock::execute_with; @@ -80,8 +80,3 @@ fn update<'a>(client: &'a Client, data: &'a UpdateData) -> LocalResponse<'a> { .body(serde_json::to_string(&data).unwrap()) .dispatch() } - -fn parse_ok_response<'a>(response: LocalResponse<'a>) -> User { - let body = response.into_string().unwrap(); - serde_json::from_str(&body).expect("can't parse value") -} \ No newline at end of file From e1939c53ad3c4c522a4d0aefbf8d7bc7d605e873 Mon Sep 17 00:00:00 2001 From: Bolaji Ahmad <56865496+bolajahmad@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:52:47 +0100 Subject: [PATCH 9/9] lint: added code formatting using cargo format --- services/api/routes/src/tests/mock.rs | 2 +- services/api/routes/src/tests/mod.rs | 2 +- services/api/routes/src/tests/update.rs | 127 ++++++++++++++---------- services/api/routes/src/update.rs | 8 +- 4 files changed, 78 insertions(+), 61 deletions(-) diff --git a/services/api/routes/src/tests/mock.rs b/services/api/routes/src/tests/mock.rs index 86f4428..9431af0 100644 --- a/services/api/routes/src/tests/mock.rs +++ b/services/api/routes/src/tests/mock.rs @@ -21,4 +21,4 @@ pub fn parse_err_response<'a>(response: LocalResponse<'a>) -> Error { let body = response.into_string().unwrap(); let error: ErrorResponse = serde_json::from_str(&body).unwrap(); error.message.into() -} \ No newline at end of file +} diff --git a/services/api/routes/src/tests/mod.rs b/services/api/routes/src/tests/mod.rs index 22ff8db..504fb56 100644 --- a/services/api/routes/src/tests/mod.rs +++ b/services/api/routes/src/tests/mod.rs @@ -1,4 +1,4 @@ mod mock; mod query; mod register; -mod update; \ No newline at end of file +mod update; diff --git a/services/api/routes/src/tests/update.rs b/services/api/routes/src/tests/update.rs index b0b3f97..ad1860b 100644 --- a/services/api/routes/src/tests/update.rs +++ b/services/api/routes/src/tests/update.rs @@ -1,8 +1,18 @@ -use rocket::{http::{ContentType, Status}, local::blocking::{Client, LocalResponse}, routes}; +use rocket::{ + http::{ContentType, Status}, + local::blocking::{Client, LocalResponse}, + routes, +}; use storage::{init_db, users::User}; use types::Notifier; -use crate::{query::user, register::{register_user, RegistrationData}, tests::mock::parse_ok_response, update::{update_user, UpdateData}, LOG_TARGET}; +use crate::{ + query::user, + register::{register_user, RegistrationData}, + tests::mock::parse_ok_response, + update::{update_user, UpdateData}, + LOG_TARGET, +}; use super::mock::execute_with; @@ -10,60 +20,68 @@ pub const DB_PATH: &'static str = "update-tests.db"; #[test] fn updating_users_works() { - execute_with(DB_PATH, || { - let conn = init_db(DB_PATH).unwrap(); - let rocket = rocket::build().manage(conn).mount("/", routes![register_user, user, update_user]); - let client = Client::tracked(rocket).expect("failed to create client"); + execute_with(DB_PATH, || { + let conn = init_db(DB_PATH).unwrap(); + let rocket = rocket::build() + .manage(conn) + .mount("/", routes![register_user, user, update_user]); + let client = Client::tracked(rocket).expect("failed to create client"); - let registration_data = RegistrationData { - id: 0, - notifier: Notifier::Telegram, - email: None, - tg_handle: Some("@dummy".to_string()), - enabled_notifications: vec![], - }; + let registration_data = RegistrationData { + id: 0, + notifier: Notifier::Telegram, + email: None, + tg_handle: Some("@dummy".to_string()), + enabled_notifications: vec![], + }; - // Should register successfully - let response = register(&client, ®istration_data); - assert_eq!(response.status(), Status::Ok); + // Should register successfully + let response = register(&client, ®istration_data); + assert_eq!(response.status(), Status::Ok); - let response = client.get("/user/0").dispatch(); - assert_eq!(parse_ok_response(response), User { - id: 0, - email: None, - tg_handle: Some("@dummy".to_string()), - notifier: Notifier::Telegram, - }); + let response = client.get("/user/0").dispatch(); + assert_eq!( + parse_ok_response(response), + User { + id: 0, + email: None, + tg_handle: Some("@dummy".to_string()), + notifier: Notifier::Telegram, + } + ); - // Update to mismatched notifier should not work - let mut update_data = UpdateData { - id: 0, - email: None, - tg_handle: Some("@dummy".to_string()), - notifier: Some(Notifier::Email), - }; - let response = update(&client, &update_data); - assert_eq!(response.status(), Status::BadRequest); + // Update to mismatched notifier should not work + let mut update_data = UpdateData { + id: 0, + email: None, + tg_handle: Some("@dummy".to_string()), + notifier: Some(Notifier::Email), + }; + let response = update(&client, &update_data); + assert_eq!(response.status(), Status::BadRequest); - // Update email with notifier works - let update_data = UpdateData { - id: 0, - email: Some("dummy@mail.com".to_string()), - tg_handle: Some("@dummy".to_string()), - notifier: Some(Notifier::Email) - }; - let response = update(&client, &update_data); - assert_eq!(response.status(), Status::Ok); + // Update email with notifier works + let update_data = UpdateData { + id: 0, + email: Some("dummy@mail.com".to_string()), + tg_handle: Some("@dummy".to_string()), + notifier: Some(Notifier::Email), + }; + let response = update(&client, &update_data); + assert_eq!(response.status(), Status::Ok); - // Should return the updated user information - let response = client.get("/user/0").dispatch(); - assert_eq!(parse_ok_response(response), User { - id: 0, - email: Some("dummy@mail.com".to_string()), - tg_handle: Some("@dummy".to_string()), - notifier: Notifier::Email, - }); - }) + // Should return the updated user information + let response = client.get("/user/0").dispatch(); + assert_eq!( + parse_ok_response(response), + User { + id: 0, + email: Some("dummy@mail.com".to_string()), + tg_handle: Some("@dummy".to_string()), + notifier: Notifier::Email, + } + ); + }) } fn register<'a>(client: &'a Client, data: &'a RegistrationData) -> LocalResponse<'a> { @@ -75,8 +93,9 @@ fn register<'a>(client: &'a Client, data: &'a RegistrationData) -> LocalResponse } fn update<'a>(client: &'a Client, data: &'a UpdateData) -> LocalResponse<'a> { - client.put("/update_user") - .header(ContentType::JSON) - .body(serde_json::to_string(&data).unwrap()) - .dispatch() + client + .put("/update_user") + .header(ContentType::JSON) + .body(serde_json::to_string(&data).unwrap()) + .dispatch() } diff --git a/services/api/routes/src/update.rs b/services/api/routes/src/update.rs index 4e75bec..4bd8d80 100644 --- a/services/api/routes/src/update.rs +++ b/services/api/routes/src/update.rs @@ -62,9 +62,7 @@ pub async fn update_user( log::info!(target: LOG_TARGET, "Update user request {:?}", update_data); // validate the data passed - update_data - .validate() - .map_err(|err| custom_error(Status::BadRequest, err))?; + update_data.validate().map_err(|err| custom_error(Status::BadRequest, err))?; // Get connection: let conn = conn.lock().map_err(|err| { @@ -84,8 +82,8 @@ pub async fn update_user( } else { db_user.notifier }, - }; - let result = User::update(&conn, &user); + }; + let result = User::update(&conn, &user); match result { Ok(_) => Ok(status::Custom(Status::Ok, ())),