From 012a3175bb67bcd79cc6742c8dab32600a6ebc8f Mon Sep 17 00:00:00 2001 From: Nicole Tietz-Sokolskaya Date: Sun, 2 Jun 2024 11:21:50 -0400 Subject: [PATCH] refactoring --- src/bin/admin.rs | 14 ++---------- src/db.rs | 4 +--- src/handler.rs | 13 +---------- src/handler/documents.rs | 6 +++--- src/handler/home.rs | 5 +++-- src/handler/login.rs | 36 ++++++++++++------------------- src/handler/projects.rs | 22 ++++++++----------- src/logging.rs | 4 +--- src/models/documents.rs | 14 ++++-------- src/models/project_memberships.rs | 5 ++--- src/models/projects.rs | 17 ++++----------- src/models/users.rs | 22 +++++-------------- src/password.rs | 8 ++----- src/permissions.rs | 19 +++++----------- src/provider.rs | 31 ++++++++++++-------------- src/session.rs | 2 +- src/templates.rs | 11 +++------- src/validation.rs | 5 +---- 18 files changed, 75 insertions(+), 163 deletions(-) diff --git a/src/bin/admin.rs b/src/bin/admin.rs index e29470b..326d003 100644 --- a/src/bin/admin.rs +++ b/src/bin/admin.rs @@ -11,12 +11,7 @@ pub async fn main() -> Result<()> { let db_url = dotenvy::var("DATABASE_URL")?; match AdminCli::parse().command { - AdminCommand::CreateUser { - name, - email, - username, - password, - } => { + AdminCommand::CreateUser { name, email, username, password } => { let password = match password { Some(p) => p, None => { @@ -43,12 +38,7 @@ struct AdminCli { #[derive(Subcommand, Debug)] pub enum AdminCommand { - CreateUser { - name: String, - email: String, - username: String, - password: Option, - }, + CreateUser { name: String, email: String, username: String, password: Option }, ListUsers, } diff --git a/src/db.rs b/src/db.rs index f8131e0..cc8dce6 100644 --- a/src/db.rs +++ b/src/db.rs @@ -26,9 +26,7 @@ pub fn establish_connection(url: &str) -> SqliteConnection { /// Panics if the connection pool cannot be created. pub fn build_connection_pool(url: &str) -> Pool> { let manager = ConnectionManager::::new(url); - Pool::builder() - .build(manager) - .expect("Failed to create connection pool.") + Pool::builder().build(manager).expect("Failed to create connection pool.") } /// Runs any pending migrations. diff --git a/src/handler.rs b/src/handler.rs index 9653742..6d220f5 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -4,24 +4,13 @@ pub mod login; pub mod projects; use axum::http::StatusCode; -use axum::response::Response; pub use login::{login_page, login_submit}; use tracing::error; -pub fn internal_server_error() -> Response { - Response::builder() - .status(StatusCode::INTERNAL_SERVER_ERROR) - .body("Internal Server Error".into()) - .unwrap() -} - pub fn internal_error(err: E) -> (StatusCode, String) where E: std::error::Error, { error!(?err, "internal error"); - ( - StatusCode::INTERNAL_SERVER_ERROR, - "Internal Server Error".into(), - ) + (StatusCode::INTERNAL_SERVER_ERROR, "Internal Server Error".into()) } diff --git a/src/handler/documents.rs b/src/handler/documents.rs index 16dfff6..e717d27 100644 --- a/src/handler/documents.rs +++ b/src/handler/documents.rs @@ -38,7 +38,7 @@ async fn render_documents_page( projects => projects, }; - Ok(provider.render_resp("documents/list_documents.html", values)) + provider.render_resp("documents/list_documents.html", values) } pub async fn create_document_page( @@ -59,7 +59,7 @@ pub async fn create_document_page( user => user, projects => projects, }; - Ok(provider.render_resp("documents/create_document.html", values)) + provider.render_resp("documents/create_document.html", values) } #[derive(Debug, Deserialize)] @@ -138,7 +138,7 @@ pub async fn edit_document_page( projects => projects, }; - Ok(provider.render_resp("documents/edit_document.html", values)) + provider.render_resp("documents/edit_document.html", values) } #[derive(Debug, Deserialize)] diff --git a/src/handler/home.rs b/src/handler/home.rs index f7bfc3d..64573d9 100644 --- a/src/handler/home.rs +++ b/src/handler/home.rs @@ -1,3 +1,4 @@ +use axum::http::StatusCode; use axum::response::Redirect; use axum_login::AuthSession; @@ -8,7 +9,7 @@ use crate::prelude::*; pub async fn home_page( State(provider): State, auth_session: AuthSession, -) -> Response { +) -> Result { if let Some(user) = auth_session.user { let mut db = provider.db_pool.get().unwrap(); let projects: Vec = @@ -21,6 +22,6 @@ pub async fn home_page( provider.render_resp("home.html", values) } else { - Redirect::to("/login").into_response() + Ok(Redirect::to("/login").into_response()) } } diff --git a/src/handler/login.rs b/src/handler/login.rs index 6dbd8e9..4dbfb25 100644 --- a/src/handler/login.rs +++ b/src/handler/login.rs @@ -1,8 +1,9 @@ +use axum::http::StatusCode; use axum::response::Redirect; use axum::Form; use axum_login::AuthSession; -use crate::handler::internal_server_error; +use super::internal_error; use crate::prelude::*; use crate::session::Credentials; @@ -15,12 +16,12 @@ pub struct LoginTemplate { pub async fn login_page( State(provider): State, auth_session: AuthSession, -) -> Response { - if auth_session.user.is_some() { - return Redirect::to("/").into_response(); +) -> Result { + if let Some(_user) = auth_session.user { + Ok(Redirect::to("/").into_response()) + } else { + render_login_page(&provider, "", "", None) } - - render_login_page(&provider, "", "", None) } fn render_login_page( @@ -28,7 +29,7 @@ fn render_login_page( username: &str, password: &str, error: Option<&'static str>, -) -> Response { +) -> Result { provider.render_resp( "login.html", context! { @@ -45,21 +46,12 @@ pub async fn login_submit( State(provider): State, mut auth_session: AuthSession, Form(creds): Form, -) -> Response { - match auth_session.authenticate(creds).await { - Ok(Some(user)) => { - if let Err(err) = auth_session.login(&user).await { - error!(?err, "error while logging in user"); - return internal_server_error(); - } - - Redirect::to("/").into_response() - } - Ok(None) => render_login_page(&provider, "", "", Some(LOGIN_ERROR_MSG)), - Err(err) => { - error!(?err, "error while authenticating user"); - internal_server_error() - } +) -> Result { + if let Some(user) = auth_session.authenticate(creds).await.map_err(internal_error)? { + let _ = auth_session.login(&user).await.map_err(internal_error)?; + Ok(Redirect::to("/").into_response()) + } else { + render_login_page(&provider, "", "", Some(LOGIN_ERROR_MSG)) } } diff --git a/src/handler/projects.rs b/src/handler/projects.rs index 2c1884c..bfbd554 100644 --- a/src/handler/projects.rs +++ b/src/handler/projects.rs @@ -4,7 +4,6 @@ use axum::Form; use axum_login::AuthSession; use super::internal_error; -use crate::handler::internal_server_error; use crate::models::project_memberships::{self, ProjectRole}; use crate::models::projects::{self, NewProject}; use crate::models::users::User; @@ -14,22 +13,19 @@ use crate::prelude::*; pub async fn projects_page( State(provider): State, auth_session: AuthSession, -) -> Response { +) -> Result { if let Some(user) = auth_session.user { render_projects_page(provider, user).await } else { - Redirect::to("/login").into_response() + Ok(Redirect::to("/login").into_response()) } } -async fn render_projects_page(provider: Provider, user: User) -> Response { - let mut db = match provider.db_pool.get() { - Ok(db) => db, - Err(err) => { - error!(?err, "failed to get db connection"); - return internal_server_error(); - } - }; +async fn render_projects_page( + provider: Provider, + user: User, +) -> Result { + let mut db = provider.db_pool.get().map_err(internal_error)?; let projects = permissions::query::accessible_projects(&mut db, &user.id).unwrap_or_default(); let values = context! { @@ -43,10 +39,10 @@ async fn render_projects_page(provider: Provider, user: User) -> Response { pub async fn create_project_page( State(provider): State, auth_session: AuthSession, -) -> Response { +) -> Result { let user = match auth_session.user { Some(user) => user, - None => return Redirect::to("/login").into_response(), + None => return Ok(Redirect::to("/login").into_response()), }; let values = context! { diff --git a/src/logging.rs b/src/logging.rs index 5e03789..a961a3d 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -1,7 +1,5 @@ use tracing_subscriber::EnvFilter; pub fn setup_logging() { - tracing_subscriber::fmt() - .with_env_filter(EnvFilter::from_default_env()) - .init(); + tracing_subscriber::fmt().with_env_filter(EnvFilter::from_default_env()).init(); } diff --git a/src/models/documents.rs b/src/models/documents.rs index a49050a..2881056 100644 --- a/src/models/documents.rs +++ b/src/models/documents.rs @@ -45,13 +45,9 @@ pub mod query { db: &mut SqliteConnection, new_document: NewDocument, ) -> Result { - diesel::insert_into(dsl::documents) - .values(&new_document) - .execute(db)?; + diesel::insert_into(dsl::documents).values(&new_document).execute(db)?; - let document = dsl::documents - .filter(dsl::id.eq(&new_document.id)) - .first(db)?; + let document = dsl::documents.filter(dsl::id.eq(&new_document.id)).first(db)?; Ok(document) } @@ -75,10 +71,8 @@ pub mod query { db: &mut SqliteConnection, document_id: &str, ) -> Result, DbError> { - let document = dsl::documents - .filter(dsl::id.eq(document_id)) - .first::(db) - .optional()?; + let document = + dsl::documents.filter(dsl::id.eq(document_id)).first::(db).optional()?; Ok(document) } diff --git a/src/models/project_memberships.rs b/src/models/project_memberships.rs index 19922c5..c60bf46 100644 --- a/src/models/project_memberships.rs +++ b/src/models/project_memberships.rs @@ -84,9 +84,8 @@ pub mod query { role, }; - let membership = diesel::insert_into(pm::project_memberships) - .values(new_membership) - .get_result(db)?; + let membership = + diesel::insert_into(pm::project_memberships).values(new_membership).get_result(db)?; Ok(membership) } diff --git a/src/models/projects.rs b/src/models/projects.rs index 20f630c..50cfb88 100644 --- a/src/models/projects.rs +++ b/src/models/projects.rs @@ -28,13 +28,7 @@ pub struct NewProject { impl NewProject { pub fn new(creator_id: String, name: String, description: String, key: String) -> Self { - Self { - id: Uuid::now_v7().to_string(), - creator_id, - name, - description, - key, - } + Self { id: Uuid::now_v7().to_string(), creator_id, name, description, key } } } @@ -42,9 +36,8 @@ pub mod query { use super::*; pub fn for_user(db: &mut SqliteConnection, user_id: String) -> Result, DbError> { - let projects = dsl::projects - .filter(dsl::creator_id.eq(user_id.to_string())) - .load::(db)?; + let projects = + dsl::projects.filter(dsl::creator_id.eq(user_id.to_string())).load::(db)?; Ok(projects) } @@ -54,9 +47,7 @@ pub mod query { ) -> Result { use crate::schema::projects::dsl as p; - let project = diesel::insert_into(p::projects) - .values(new_project) - .get_result(db)?; + let project = diesel::insert_into(p::projects).values(new_project).get_result(db)?; Ok(project) } diff --git a/src/models/users.rs b/src/models/users.rs index 40d97f9..a16704e 100644 --- a/src/models/users.rs +++ b/src/models/users.rs @@ -3,9 +3,9 @@ use serde::Serialize; use uuid::Uuid; use super::DbError; -use crate::db::ValidationError; use crate::password; use crate::schema::users::dsl; +use crate::validation::ValidationError; #[derive(Queryable, Selectable, Debug, Clone, Serialize)] #[diesel(table_name = crate::schema::users)] @@ -31,13 +31,7 @@ pub struct NewUser { impl NewUser { pub fn new(name: String, username: String, email: String, password: String) -> Self { let password_hash = password::hash(&password); - Self { - id: Uuid::now_v7().to_string(), - name, - username, - email, - password_hash, - } + Self { id: Uuid::now_v7().to_string(), name, username, email, password_hash } } pub fn validate(&self) -> Result<(), Vec> { @@ -82,20 +76,14 @@ impl<'a> Query<'a> { } pub fn by_username(&mut self, username: &str) -> Result { - let user = dsl::users - .filter(dsl::username.eq(username)) - .first::(self.db)?; + let user = dsl::users.filter(dsl::username.eq(username)).first::(self.db)?; Ok(user) } pub fn create(&mut self, new_user: NewUser) -> Result { - let _ = diesel::insert_into(dsl::users) - .values(&new_user) - .execute(self.db)?; + let _ = diesel::insert_into(dsl::users).values(&new_user).execute(self.db)?; - let new_user = dsl::users - .filter(dsl::id.eq(&new_user.id)) - .first::(self.db)?; + let new_user = dsl::users.filter(dsl::id.eq(&new_user.id)).first::(self.db)?; Ok(new_user) } diff --git a/src/password.rs b/src/password.rs index e2bebc7..68dc061 100644 --- a/src/password.rs +++ b/src/password.rs @@ -9,18 +9,14 @@ pub fn verify(hash: &str, password: &str) -> bool { Err(_) => return false, // TODO: log an error }; - Argon2::default() - .verify_password(password.as_bytes(), &parsed_hash) - .is_ok() + Argon2::default().verify_password(password.as_bytes(), &parsed_hash).is_ok() } /// Hashes the given password. pub fn hash(password: &str) -> String { let salt = SaltString::generate(&mut OsRng); - let hash = Argon2::default() - .hash_password(password.as_bytes(), &salt) - .unwrap(); + let hash = Argon2::default().hash_password(password.as_bytes(), &salt).unwrap(); hash.to_string() } diff --git a/src/permissions.rs b/src/permissions.rs index 6cd7f28..5db3848 100644 --- a/src/permissions.rs +++ b/src/permissions.rs @@ -49,10 +49,8 @@ pub mod query { ) -> Result { use crate::schema::documents::dsl as d; - let document = d::documents - .filter(d::id.eq(document_id)) - .first::(db) - .optional()?; + let document = + d::documents.filter(d::id.eq(document_id)).first::(db).optional()?; match document { Some(doc) => check_user_project(db, user_id, &doc.project_id, permission), @@ -83,9 +81,7 @@ pub mod query { use crate::schema::projects::dsl as p; let project_ids = accessible_project_ids(db, user_id)?; - let projects = p::projects - .filter(p::id.eq_any(project_ids)) - .load::(db)?; + let projects = p::projects.filter(p::id.eq_any(project_ids)).load::(db)?; Ok(projects) } @@ -111,10 +107,7 @@ pub mod query { .select(d::id) .load::(db)?; - let document_ids = direct_documents - .into_iter() - .chain(project_documents) - .collect(); + let document_ids = direct_documents.into_iter().chain(project_documents).collect(); Ok(document_ids) } @@ -128,9 +121,7 @@ pub mod query { use crate::schema::documents::dsl as d; let document_ids = accessible_document_ids(db, user_id)?; - let documents = d::documents - .filter(d::id.eq_any(document_ids)) - .load::(db)?; + let documents = d::documents.filter(d::id.eq_any(document_ids)).load::(db)?; Ok(documents) } diff --git a/src/provider.rs b/src/provider.rs index 677d416..8016fbd 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -1,11 +1,12 @@ use std::sync::Arc; +use axum::http::StatusCode; use diesel::r2d2::{ConnectionManager, Pool}; use diesel::SqliteConnection; use minijinja_autoreload::AutoReloader; use thiserror::Error; -use crate::handler::internal_server_error; +use crate::handler::internal_error; use crate::prelude::*; pub type ConnectionPool = Pool>; @@ -21,14 +22,14 @@ pub struct Provider { pub enum ProviderError { #[error("Error while using the connection pool: {0}")] R2D2Error(#[from] diesel::r2d2::PoolError), + + #[error("Error while rendering template: {0}")] + TemplateError(#[from] minijinja::Error), } impl Provider { pub fn new(db: ConnectionPool, template_loader: AutoReloader) -> Provider { - Provider { - db_pool: db, - template_loader: Arc::new(template_loader), - } + Provider { db_pool: db, template_loader: Arc::new(template_loader) } } pub fn db_conn(&self) -> Result { @@ -36,23 +37,19 @@ impl Provider { Ok(conn) } - pub fn render(&self, path: &str, data: T) -> anyhow::Result { - // TODO: more graceful handling of the potential errors here; this should not - // use anyhow + pub fn render(&self, path: &str, data: T) -> Result { let env = self.template_loader.acquire_env().unwrap(); let template = env.get_template(path)?; let rendered = template.render(data)?; Ok(rendered) } - pub fn render_resp(&self, path: &str, data: T) -> Response { - let rendered = self.render(path, data); - match rendered { - Ok(rendered) => Html(rendered).into_response(), - Err(err) => { - error!(?err, "error while rendering template"); - internal_server_error() - } - } + pub fn render_resp( + &self, + path: &str, + data: T, + ) -> Result { + let rendered = self.render(path, data).map_err(internal_error)?; + Ok(Html(rendered).into_response()) } } diff --git a/src/session.rs b/src/session.rs index 5b08043..05c42f0 100644 --- a/src/session.rs +++ b/src/session.rs @@ -5,7 +5,7 @@ use crate::models::{self, users, DbError}; use crate::password; use crate::prelude::*; -#[derive(Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct Credentials { pub username: String, pub password: String, diff --git a/src/templates.rs b/src/templates.rs index ac08745..9e45fb5 100644 --- a/src/templates.rs +++ b/src/templates.rs @@ -24,13 +24,8 @@ pub fn setup_filters(env: &mut Environment) { pub fn heroicon_filter(name: String, classes: Option) -> Result { let class = classes.unwrap_or_else(|| "".to_owned()); - let attrs = IconAttrs::default() - .class(&class) - .fill("none") - .stroke_color("currentColor"); + let attrs = IconAttrs::default().class(&class).fill("none").stroke_color("currentColor"); - free_icons::heroicons(&name, true, attrs).ok_or(Error::new( - ErrorKind::TemplateNotFound, - "cannot find template for requested icon", - )) + free_icons::heroicons(&name, true, attrs) + .ok_or(Error::new(ErrorKind::TemplateNotFound, "cannot find template for requested icon")) } diff --git a/src/validation.rs b/src/validation.rs index 7eb4264..19946b0 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -8,9 +8,6 @@ pub struct ValidationError { impl ValidationError { pub fn on(field: &str, message: &str) -> ValidationError { - ValidationError { - field: field.to_owned(), - message: message.to_owned(), - } + ValidationError { field: field.to_owned(), message: message.to_owned() } } }