Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update user logic #13

Merged
merged 9 commits into from
Sep 5, 2024
2 changes: 1 addition & 1 deletion macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
macro_rules! ensure {
( $x:expr, $y:expr $(,)? ) => {{
if !$x {
return Err($y)
return Err($y);
}
}};
}
1 change: 1 addition & 0 deletions services/api/routes/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
17 changes: 17 additions & 0 deletions services/api/routes/src/tests/mock.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
use rocket::local::blocking::LocalResponse;
use storage::users::User;
use types::api::ErrorResponse;

use crate::errors::Error;

pub fn execute_with<R>(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.
let _ = std::fs::remove_file(db_path);

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()
}
1 change: 1 addition & 0 deletions services/api/routes/src/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod mock;
mod query;
mod register;
mod update;
13 changes: 1 addition & 12 deletions services/api/routes/src/tests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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()
}
13 changes: 1 addition & 12 deletions services/api/routes/src/tests/register.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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()
}
101 changes: 101 additions & 0 deletions services/api/routes/src/tests/update.rs
Original file line number Diff line number Diff line change
@@ -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, &registration_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("[email protected]".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("[email protected]".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()
}
106 changes: 106 additions & 0 deletions services/api/routes/src/update.rs
Original file line number Diff line number Diff line change
@@ -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<String>,
#[serde(rename = "tgHandle")]
// The telegram handle to update to
pub tg_handle: Option<String>,
// 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<Notifier>,
}

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 = "<update_data>")]
pub async fn update_user(
conn: &State<DbConn>,
update_data: Json<UpdateData>,
) -> Result<status::Custom<()>, status::Custom<Json<ErrorResponse>>> {
// 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<User, status::Custom<Json<ErrorResponse>>> {
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)),
}
}
4 changes: 2 additions & 2 deletions services/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -20,7 +20,7 @@ pub async fn rocket() -> Rocket<Build> {
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,
Expand Down
37 changes: 30 additions & 7 deletions services/storage/src/users.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rusqlite::{params, Connection, Result};
use rusqlite::{params, Connection, Error, Result};
use serde::{Deserialize, Serialize};
use types::Notifier;

Expand Down Expand Up @@ -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],
Expand All @@ -136,4 +132,31 @@ impl User {
};
Ok(())
}

pub fn update(conn: &Connection, user: &User) -> Result<usize, Error> {
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<String> {
match notifier {
Notifier::Email => Some(String::from("email")),
Notifier::Telegram => Some("telegram".to_string()),
_ => None,
}
}
}