diff --git a/link-for-later/src/controller/error.rs b/link-for-later/src/controller/error.rs index 97a0428..37b476b 100644 --- a/link-for-later/src/controller/error.rs +++ b/link-for-later/src/controller/error.rs @@ -7,6 +7,7 @@ use serde_json::json; use crate::types::AppError; +#[allow(clippy::cognitive_complexity)] impl IntoResponse for AppError { fn into_response(self) -> Response { let (status, error_message) = match self { @@ -18,16 +19,30 @@ impl IntoResponse for AppError { tracing::debug!("Database error: {}", e.to_string()); (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()) } - Self::LinkNotFound => (StatusCode::NOT_FOUND, self.to_string()), - Self::UserAlreadyExists - | Self::UserNotFound - | Self::InvalidEmail - | Self::InvalidUrl => (StatusCode::BAD_REQUEST, self.to_string()), - Self::IncorrectPassword => (StatusCode::UNAUTHORIZED, self.to_string()), + Self::LinkNotFound(ref e) => { + tracing::debug!("Link not found: {}", e.to_string()); + (StatusCode::NOT_FOUND, self.to_string()) + } + Self::UserAlreadyExists(ref e) => { + tracing::debug!("User already exists: {}", e.to_string()); + (StatusCode::BAD_REQUEST, self.to_string()) + } + Self::UserNotFound(ref e) => { + tracing::debug!("User not found: {}", e.to_string()); + (StatusCode::BAD_REQUEST, self.to_string()) + } + Self::IncorrectPassword(ref e) => { + tracing::debug!("Incorrect password: {}", e.to_string()); + (StatusCode::UNAUTHORIZED, self.to_string()) + } Self::AuthorizationError(ref e) => { tracing::debug!("Authorization error: {}", e.to_string()); (StatusCode::UNAUTHORIZED, self.to_string()) } + Self::ValidationError(ref e) => { + tracing::debug!("Payload validation error: {}", e.to_string()); + (StatusCode::BAD_REQUEST, self.to_string()) + } #[cfg(test)] Self::TestError => (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()), @@ -49,46 +64,52 @@ mod tests { #[test] fn test_error_response() { assert_eq!( - AppError::ServerError("server error occurred".into()) + AppError::ServerError("a server operation failed".into()) .into_response() .status(), StatusCode::INTERNAL_SERVER_ERROR ); assert_eq!( - AppError::DatabaseError("database error occurred".into()) + AppError::DatabaseError("a database operation failed".into()) .into_response() .status(), StatusCode::INTERNAL_SERVER_ERROR ); assert_eq!( - AppError::LinkNotFound.into_response().status(), + AppError::LinkNotFound("link".into()) + .into_response() + .status(), StatusCode::NOT_FOUND ); assert_eq!( - AppError::UserAlreadyExists.into_response().status(), - StatusCode::BAD_REQUEST - ); - assert_eq!( - AppError::UserNotFound.into_response().status(), + AppError::UserAlreadyExists("user".into()) + .into_response() + .status(), StatusCode::BAD_REQUEST ); assert_eq!( - AppError::InvalidEmail.into_response().status(), + AppError::UserNotFound("user".into()) + .into_response() + .status(), StatusCode::BAD_REQUEST ); assert_eq!( - AppError::InvalidUrl.into_response().status(), - StatusCode::BAD_REQUEST + AppError::IncorrectPassword("user".into()) + .into_response() + .status(), + StatusCode::UNAUTHORIZED ); assert_eq!( - AppError::IncorrectPassword.into_response().status(), + AppError::AuthorizationError("an authorization error occurred".into()) + .into_response() + .status(), StatusCode::UNAUTHORIZED ); assert_eq!( - AppError::AuthorizationError("authorization error occurred".into()) + AppError::ValidationError("a validation error occurred".into()) .into_response() .status(), - StatusCode::UNAUTHORIZED + StatusCode::BAD_REQUEST ); } } diff --git a/link-for-later/src/controller/links.rs b/link-for-later/src/controller/links.rs index a348397..f058feb 100644 --- a/link-for-later/src/controller/links.rs +++ b/link-for-later/src/controller/links.rs @@ -53,8 +53,7 @@ async fn post( match payload.validate() { Ok(()) => {} Err(e) => { - tracing::error!("Error: {}", e); - return AppError::InvalidUrl.into_response(); + return AppError::ValidationError(format!("post_link() {e:?}")).into_response(); } } @@ -105,8 +104,7 @@ async fn put( match payload.validate() { Ok(()) => {} Err(e) => { - tracing::error!("Error: {}", e); - return AppError::InvalidUrl.into_response(); + return AppError::ValidationError(format!("put_link() {e:?}")).into_response(); } } @@ -301,7 +299,7 @@ mod tests { let body = body.collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid url"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); } #[traced_test] @@ -458,7 +456,7 @@ mod tests { let body = body.collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid url"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); } #[traced_test] diff --git a/link-for-later/src/controller/users.rs b/link-for-later/src/controller/users.rs index 5b41f0d..851b3d3 100644 --- a/link-for-later/src/controller/users.rs +++ b/link-for-later/src/controller/users.rs @@ -31,8 +31,7 @@ async fn register( match payload.validate() { Ok(()) => {} Err(e) => { - tracing::error!("Error: {}", e); - return AppError::InvalidEmail.into_response(); + return AppError::ValidationError(format!("register() {e:?}")).into_response(); } } @@ -58,8 +57,7 @@ async fn login( match payload.validate() { Ok(()) => {} Err(e) => { - tracing::error!("Error: {}", e); - return AppError::InvalidEmail.into_response(); + return AppError::ValidationError(format!("login() {e:?}")).into_response(); } } @@ -141,7 +139,7 @@ mod tests { let body = body.collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid email"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); } #[traced_test] @@ -209,7 +207,7 @@ mod tests { let body = body.collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid email"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); } #[traced_test] diff --git a/link-for-later/src/repository/inmemory.rs b/link-for-later/src/repository/inmemory.rs index d69599a..cb66fab 100644 --- a/link-for-later/src/repository/inmemory.rs +++ b/link-for-later/src/repository/inmemory.rs @@ -46,7 +46,7 @@ impl LinksRepository for LinksRepositoryProvider { .iter() .find(|link| link.id() == query.id() && link.owner() == query.owner()) .cloned() - .ok_or(AppError::LinkNotFound) + .ok_or_else(|| AppError::LinkNotFound(query.id().to_owned())) } async fn create(&self, item: &LinkItem) -> Result { @@ -66,7 +66,7 @@ impl LinksRepository for LinksRepositoryProvider { .iter() .find(|link| link.id() == id && link.owner() == item.owner()) .cloned() - .ok_or(AppError::LinkNotFound)?; + .ok_or_else(|| AppError::LinkNotFound(id.to_owned()))?; self.delete(item).await?; INMEMORY_LINKS_DATA.lock().unwrap().push(item.clone()); Ok(item.clone()) @@ -92,7 +92,7 @@ impl UsersRepository for UsersRepositoryProvider { .iter() .find(|user| user.email() == query.email()) .cloned() - .ok_or(AppError::UserNotFound) + .ok_or_else(|| AppError::UserNotFound(query.email().to_owned())) } async fn create(&self, info: &UserInfo) -> Result { diff --git a/link-for-later/src/repository/mongodb.rs b/link-for-later/src/repository/mongodb.rs index 36de5bf..f92a829 100644 --- a/link-for-later/src/repository/mongodb.rs +++ b/link-for-later/src/repository/mongodb.rs @@ -65,7 +65,7 @@ impl LinksRepository for LinksRepositoryProvider { .find_one(db_query, None) .await .map_err(|e| AppError::DatabaseError(format!("find_one() {e:?}")))?; - item.ok_or(AppError::LinkNotFound) + item.ok_or_else(|| AppError::LinkNotFound(query.id().to_owned())) } async fn create(&self, item: &LinkItem) -> Result { @@ -123,7 +123,7 @@ impl UsersRepository for UsersRepositoryProvider { .find_one(db_query, None) .await .map_err(|e| AppError::DatabaseError(format!("find_one() {e:?}")))?; - item.ok_or(AppError::UserNotFound) + item.ok_or_else(|| AppError::UserNotFound(query.email().to_owned())) } async fn create(&self, info: &UserInfo) -> Result { diff --git a/link-for-later/src/service/links.rs b/link-for-later/src/service/links.rs index b93397a..d5c636b 100644 --- a/link-for-later/src/service/links.rs +++ b/link-for-later/src/service/links.rs @@ -193,14 +193,14 @@ mod tests { .expect_get() .withf(move |query| query == &repo_query) .times(1) - .returning(|_| Err(AppError::LinkNotFound)); + .returning(|_| Err(AppError::LinkNotFound("1".into()))); let links_service = ServiceProvider {}; let response = links_service .get(Box::new(Arc::new(mock_links_repo)), &request_query) .await; - assert_eq!(response, Err(AppError::LinkNotFound)); + assert_eq!(response, Err(AppError::LinkNotFound("1".into()))); } #[tokio::test] @@ -307,7 +307,7 @@ mod tests { .expect_get() .withf(move |query| query == &repo_query) .times(1) - .returning(|_| Err(AppError::LinkNotFound)); + .returning(|_| Err(AppError::LinkNotFound("1".into()))); mock_links_repo .expect_update() //.withf(move |item| item == &item_to_update) @@ -318,7 +318,7 @@ mod tests { .update(Box::new(Arc::new(mock_links_repo)), "1", &request_item) .await; - assert_eq!(response, Err(AppError::LinkNotFound)); + assert_eq!(response, Err(AppError::LinkNotFound("1".into()))); } #[tokio::test] @@ -409,7 +409,7 @@ mod tests { .expect_get() .withf(move |query| query == &repo_query) .times(1) - .returning(|_| Err(AppError::LinkNotFound)); + .returning(|_| Err(AppError::LinkNotFound("1".into()))); mock_links_repo .expect_delete() //.withf(move |item| item == &item_to_delete) @@ -420,7 +420,7 @@ mod tests { .delete(Box::new(Arc::new(mock_links_repo)), &request_item) .await; - assert_eq!(response, Err(AppError::LinkNotFound)); + assert_eq!(response, Err(AppError::LinkNotFound("1".into()))); } #[tokio::test] diff --git a/link-for-later/src/service/users.rs b/link-for-later/src/service/users.rs index a4e067f..5fe8618 100644 --- a/link-for-later/src/service/users.rs +++ b/link-for-later/src/service/users.rs @@ -27,8 +27,8 @@ impl UsersService for ServiceProvider { ) -> Result { let user_query = UserQueryBuilder::new(user_info.email()).build(); let user_info = match users_repo.get(&user_query).await { - Ok(_) => return Err(AppError::UserAlreadyExists), - Err(AppError::UserNotFound) => user_info.clone(), + Ok(_) => return Err(AppError::UserAlreadyExists(user_info.email().to_owned())), + Err(AppError::UserNotFound(_)) => user_info.clone(), Err(e) => return Err(e), }; @@ -52,8 +52,7 @@ impl UsersService for ServiceProvider { let retrieved_user_info = users_repo.get(&user_query).await?; if retrieved_user_info.password() != user_info.password() { - tracing::info!("invalid password for user {}", &user_info.email()); - return Err(AppError::IncorrectPassword); + return Err(AppError::IncorrectPassword(user_info.email().to_owned())); } let timestamp = |timestamp: DateTime| -> Result { @@ -106,7 +105,7 @@ mod tests { .expect_get() .withf(move |query| query == &repo_query) .times(1) - .returning(|_| Err(AppError::UserNotFound)); + .returning(|_| Err(AppError::UserNotFound("user@test.com".into()))); mock_users_repo .expect_create() //.withf(move |user| user == &user_to_register) @@ -142,7 +141,10 @@ mod tests { .register(Box::new(Arc::new(mock_users_repo)), &request_item) .await; - assert_eq!(response, Err(AppError::UserAlreadyExists)); + assert_eq!( + response, + Err(AppError::UserAlreadyExists("user@test.com".into())) + ); } #[tokio::test] @@ -178,7 +180,7 @@ mod tests { .expect_get() .withf(move |query| query == &repo_query) .times(1) - .returning(|_| Err(AppError::UserNotFound)); + .returning(|_| Err(AppError::UserNotFound("user@test.com".into()))); mock_users_repo .expect_create() //.withf(move |user| user == &user_to_register) @@ -226,14 +228,17 @@ mod tests { .expect_get() .withf(move |query| query == &repo_query) .times(1) - .returning(move |_| Err(AppError::UserNotFound)); + .returning(move |_| Err(AppError::UserNotFound("user@test.com".into()))); let users_service = ServiceProvider {}; let response = users_service .login(Box::new(Arc::new(mock_users_repo)), &request_item) .await; - assert_eq!(response, Err(AppError::UserNotFound)); + assert_eq!( + response, + Err(AppError::UserNotFound("user@test.com".into())) + ); } #[tokio::test] @@ -255,6 +260,9 @@ mod tests { .login(Box::new(Arc::new(mock_users_repo)), &request_item) .await; - assert_eq!(response, Err(AppError::IncorrectPassword)); + assert_eq!( + response, + Err(AppError::IncorrectPassword("user@test.com".into())) + ); } } diff --git a/link-for-later/src/types/errors.rs b/link-for-later/src/types/errors.rs index 12a181e..0ac2723 100644 --- a/link-for-later/src/types/errors.rs +++ b/link-for-later/src/types/errors.rs @@ -4,13 +4,12 @@ use std::{error, fmt}; pub enum App { ServerError(String), DatabaseError(String), - LinkNotFound, - UserAlreadyExists, - UserNotFound, - IncorrectPassword, + LinkNotFound(String), + UserAlreadyExists(String), + UserNotFound(String), + IncorrectPassword(String), AuthorizationError(String), - InvalidEmail, - InvalidUrl, + ValidationError(String), #[cfg(test)] TestError, @@ -21,13 +20,12 @@ impl fmt::Display for App { match self { Self::ServerError(_) => write!(f, "server error"), Self::DatabaseError(_) => write!(f, "database error"), - Self::LinkNotFound => write!(f, "link item not found"), - Self::UserAlreadyExists => write!(f, "user already regisered"), - Self::UserNotFound => write!(f, "user not found"), - Self::IncorrectPassword => write!(f, "incorrect password for user"), + Self::LinkNotFound(_) => write!(f, "link item not found"), + Self::UserAlreadyExists(_) => write!(f, "user already registered"), + Self::UserNotFound(_) => write!(f, "user not found"), + Self::IncorrectPassword(_) => write!(f, "incorrect password for user"), Self::AuthorizationError(_) => write!(f, "invalid authorization token"), - Self::InvalidEmail => write!(f, "invalid email"), - Self::InvalidUrl => write!(f, "invalid url"), + Self::ValidationError(_) => write!(f, "invalid request"), #[cfg(test)] Self::TestError => write!(f, "test error"), @@ -45,28 +43,36 @@ mod tests { #[test] fn test_error_messages() { assert_eq!( - AppError::ServerError("server error occurred".into()).to_string(), + AppError::ServerError("a server operation failed".into()).to_string(), "server error" ); assert_eq!( - AppError::DatabaseError("database error occurred".into()).to_string(), + AppError::DatabaseError("a database operation failed".into()).to_string(), "database error" ); - assert_eq!(AppError::LinkNotFound.to_string(), "link item not found"); assert_eq!( - AppError::UserAlreadyExists.to_string(), - "user already regisered" + AppError::LinkNotFound("link".into()).to_string(), + "link item not found" ); - assert_eq!(AppError::UserNotFound.to_string(), "user not found"); - assert_eq!(AppError::InvalidEmail.to_string(), "invalid email"); - assert_eq!(AppError::InvalidUrl.to_string(), "invalid url"); assert_eq!( - AppError::IncorrectPassword.to_string(), + AppError::UserAlreadyExists("user".into()).to_string(), + "user already registered" + ); + assert_eq!( + AppError::UserNotFound("user".into()).to_string(), + "user not found" + ); + assert_eq!( + AppError::IncorrectPassword("user".into()).to_string(), "incorrect password for user" ); assert_eq!( - AppError::AuthorizationError("authorization error occurred".into()).to_string(), + AppError::AuthorizationError("an authorization error occurred".into()).to_string(), "invalid authorization token" ); + assert_eq!( + AppError::ValidationError("invalid email".into()).to_string(), + "invalid request" + ); } } diff --git a/link-for-later/tests/links.rs b/link-for-later/tests/links.rs index a109780..2b153df 100644 --- a/link-for-later/tests/links.rs +++ b/link-for-later/tests/links.rs @@ -209,7 +209,7 @@ async fn test_post_link_invalid_url(#[values(DatabaseType::MongoDb)] db_type: Da let body = response.into_body().collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid url"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); let db_count = repository.count_links().await; assert!(db_count == 0); @@ -290,7 +290,7 @@ async fn test_put_link_invalid_url(#[values(DatabaseType::MongoDb)] db_type: Dat let body = response.into_body().collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid url"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); let db_count = repository.count_links().await; assert!(db_count == 1); diff --git a/link-for-later/tests/users.rs b/link-for-later/tests/users.rs index aa56887..a635263 100644 --- a/link-for-later/tests/users.rs +++ b/link-for-later/tests/users.rs @@ -80,7 +80,7 @@ async fn test_register_user_invalid_email(#[values(DatabaseType::MongoDb)] db_ty let body = response.into_body().collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid email"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); let db_count = repository.count_users().await; assert!(db_count == 0); @@ -117,7 +117,10 @@ async fn test_register_user_already_registered( let body = response.into_body().collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "user already regisered"}).to_string()); + assert_eq!( + body, + json!({"error": "user already registered"}).to_string() + ); let db_count = repository.count_users().await; assert!(db_count == 1); @@ -184,7 +187,7 @@ async fn test_login_user_invalid_email(#[values(DatabaseType::MongoDb)] db_type: let body = response.into_body().collect().await.unwrap().to_bytes(); let body = std::str::from_utf8(&body).unwrap(); - assert_eq!(body, json!({"error": "invalid email"}).to_string()); + assert_eq!(body, json!({"error": "invalid request"}).to_string()); } #[rstest]