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/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/mock.rs b/services/api/routes/src/tests/mock.rs index 75764a4..9431af0 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() +} diff --git a/services/api/routes/src/tests/mod.rs b/services/api/routes/src/tests/mod.rs index da49b26..504fb56 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; 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 new file mode 100644 index 0000000..ad1860b --- /dev/null +++ b/services/api/routes/src/tests/update.rs @@ -0,0 +1,101 @@ +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 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() +} diff --git a/services/api/routes/src/update.rs b/services/api/routes/src/update.rs index 8b13789..4bd8d80 100644 --- a/services/api/routes/src/update.rs +++ b/services/api/routes/src/update.rs @@ -1 +1,107 @@ +//! ## 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}, + 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, + #[serde(rename = "tgHandle")] + // 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, +} + +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>> { + // 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); + custom_error(Status::InternalServerError, Error::DbConnectionFailed) + })?; + + // Ensure user exists + 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: 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( + 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 search 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)), + } +} 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, diff --git a/services/storage/src/users.rs b/services/storage/src/users.rs index aa8a091..773c3ff 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; @@ -108,17 +108,13 @@ 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) => { conn.execute( "INSERT INTO users - (id, email, tg_handle, notifier) + (email, tg_handle, notifier) VALUES (?1, ?2, ?3, ?4) ", params![id, email, tg_handle, notifier], @@ -136,4 +132,31 @@ 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); + + 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, id], + ) + } else { + conn.execute( + "UPDATE users SET email = ?1, tg_handle = ?2, notifier = NULL WHERE id = ?3", + params![email, tg_handle, id], + ) + }; + + result + } + + fn notifier_to_text(notifier: &Notifier) -> Option { + match notifier { + Notifier::Email => Some(String::from("email")), + Notifier::Telegram => Some("telegram".to_string()), + _ => None, + } + } }