From e7eb3d70707b93cb9f72c60f32573fd38044cf91 Mon Sep 17 00:00:00 2001 From: Alex Pitsikoulis Date: Wed, 24 Apr 2024 18:14:24 -0700 Subject: [PATCH] added hard and soft delete for servers --- src/handlers/server/delete.rs | 84 +++++++++++++++++++ src/handlers/server/mod.rs | 2 + src/startup.rs | 7 +- src/storage/server.rs | 43 ++++++++++ tests/api/server/create.rs | 5 +- tests/api/server/delete.rs | 153 ++++++++++++++++++++++++++++++++++ tests/api/server/mod.rs | 1 + tests/api/server/update.rs | 9 +- tests/api/user/delete.rs | 2 +- tests/utils/db/server.rs | 4 +- 10 files changed, 304 insertions(+), 6 deletions(-) create mode 100644 src/handlers/server/delete.rs create mode 100644 tests/api/server/delete.rs diff --git a/src/handlers/server/delete.rs b/src/handlers/server/delete.rs new file mode 100644 index 0000000..c0e554a --- /dev/null +++ b/src/handlers/server/delete.rs @@ -0,0 +1,84 @@ +use actix_web::{ + web::{Data, Path}, + HttpResponse, +}; +use chrono::Utc; +use sqlx::PgPool; +use uuid::Uuid; + +use crate::storage::{get_server_by_id, hard_delete_server, soft_delete_server}; + +#[tracing::instrument( + name = "Soft Deleting Server", + skip(server_id, db_pool), + fields( + id = %server_id, + ) +)] +pub async fn soft_delete(server_id: Path, db_pool: Data) -> HttpResponse { + let now = Utc::now(); + let id = server_id.into_inner(); + + match get_server_by_id(&db_pool, id).await { + Ok(server) => { + if server.deleted_at().is_some() { + let err = format!("server {} has already been soft deleted", id); + tracing::error!(err); + HttpResponse::BadRequest().body(err) + } else { + match soft_delete_server(&db_pool, id, now).await { + Ok(_) => { + tracing::info!("server {} successfully soft deleted", id); + HttpResponse::Ok().finish() + } + Err(e) => { + let err = format!("failed to soft delete server {}: {}", id, e); + tracing::error!(err); + HttpResponse::InternalServerError().finish() + } + } + } + } + Err(e) => match e { + sqlx::Error::RowNotFound => { + let err = format!("server {} not found", id); + tracing::error!(err); + HttpResponse::NotFound().body(err) + } + e => { + let err = format!("failed to soft delete server {}: {}", id, e); + tracing::error!(err); + HttpResponse::InternalServerError().body(err) + } + }, + } +} + +#[tracing::instrument( + name = "Hard Deleting Server", + skip(server_id, db_pool), + fields( + id = %server_id, + ) +)] +pub async fn hard_delete(server_id: Path, db_pool: Data) -> HttpResponse { + let id = server_id.into_inner(); + match hard_delete_server(&db_pool, id).await { + Ok(_) => { + tracing::info!("server {} successfully hard deleted", id); + HttpResponse::Ok().finish() + } + Err(e) => match e { + sqlx::Error::RowNotFound => { + let err = format!("server {} not found", id); + tracing::error!(err); + HttpResponse::NotFound().body(err) + } + e => { + let err = format!("failed to hard delete server {}: {}", id, e); + tracing::error!(err); + HttpResponse::InternalServerError().body(err) + } + }, + } +} diff --git a/src/handlers/server/mod.rs b/src/handlers/server/mod.rs index e5e44ad..41f7383 100644 --- a/src/handlers/server/mod.rs +++ b/src/handlers/server/mod.rs @@ -1,7 +1,9 @@ mod create; +mod delete; mod update; pub use create::*; +pub use delete::*; pub use update::*; pub const BASE_PATH: &str = "/servers"; diff --git a/src/startup.rs b/src/startup.rs index 9fb73bc..276c3d9 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -83,7 +83,12 @@ impl App { .service( scope(server::BASE_PATH) .route("", post().to(server::create)) - .service(scope("/{server_id}").route("", put().to(server::update))), + .service( + scope("/{server_id}") + .route("", put().to(server::update)) + .route("", delete().to(server::soft_delete)) + .route("/hard", delete().to(server::hard_delete)), + ), ) .service( scope("/ws") diff --git a/src/storage/server.rs b/src/storage/server.rs index ff36a53..c2f1425 100644 --- a/src/storage/server.rs +++ b/src/storage/server.rs @@ -1,4 +1,5 @@ use crate::domain::server::Server; +use chrono::{DateTime, Utc}; use sqlx::{postgres::PgQueryResult, query, query_as, Error, PgPool}; use uuid::Uuid; @@ -86,3 +87,45 @@ pub async fn get_many_servers_by_owner_id( .fetch_all(db_pool) .await } + +#[tracing::instrument( + name = "Soft Deleting Server in Database", + skip(server_id, deleted_at, db_pool), + fields( + server_id = %server_id, + deleted_at = %deleted_at, + ) +)] +pub async fn soft_delete_server( + db_pool: &PgPool, + server_id: Uuid, + deleted_at: DateTime, +) -> Result { + query( + r#" + UPDATE servers SET deleted_at = $1 WHERE id = $2; + "#, + ) + .bind(deleted_at) + .bind(server_id) + .execute(db_pool) + .await +} + +#[tracing::instrument( + name = "Hard Deleting Server in Database", + skip(server_id, db_pool), + fields( + server_id = %server_id, + ) +)] +pub async fn hard_delete_server(db_pool: &PgPool, server_id: Uuid) -> Result { + query( + r#" + DELETE FROM servers WHERE id = $1; + "#, + ) + .bind(server_id) + .execute(db_pool) + .await +} diff --git a/tests/api/server/create.rs b/tests/api/server/create.rs index d11c256..5d04814 100644 --- a/tests/api/server/create.rs +++ b/tests/api/server/create.rs @@ -74,7 +74,10 @@ async fn test_create_server_success() { let id = Uuid::parse_str(&response.text().await.expect("response body was empty")) .expect("response was not a valid UUID"); - let server = app.database.get_server_by_id(id).await; + let server = match app.database.get_server_by_id(id).await { + Ok(server) => server, + Err(e) => panic!("failed to retrieve server {} from database: {}", id, e), + }; assert_eq!(body, server) } diff --git a/tests/api/server/delete.rs b/tests/api/server/delete.rs new file mode 100644 index 0000000..c6839db --- /dev/null +++ b/tests/api/server/delete.rs @@ -0,0 +1,153 @@ +use crate::utils::{ + app::TestApp, + http_client::{ContentType, Header, Path}, +}; +use chrono::{Days, Utc}; +use claim::{assert_err, assert_none, assert_some}; +use muttr_server::handlers::server::BASE_PATH; +use uuid::Uuid; + +#[actix::test] +async fn test_soft_delete_success() { + let mut app = TestApp::spawn().await; + + let user = app + .database + .insert_user("testuser@email.com", "test.user", true) + .await; + let mut server = app.database.insert_server(user.id()).await; + + assert_none!( + server.deleted_at(), + "Server not initialized with None value for deleted_at", + ); + + let now = Utc::now(); + let response = app + .client + .request( + Path::DELETE(format!("{}/{}", BASE_PATH, server.id())), + &[Header::ContentType(ContentType::Json)], + None::, + ) + .await; + + assert_eq!( + 200, + response.status(), + "The API did not return 200 on valid server soft delete", + ); + + server = match app.database.get_server_by_id(server.id()).await { + Ok(server) => server, + Err(e) => panic!( + "failed to retrieve server {} from database: {}", + server.id(), + e + ), + }; + let deleted_at = assert_some!(server.deleted_at(), "Server deleted_at is None"); + let day = Days::new(1); + assert!( + deleted_at > now.checked_sub_days(day).unwrap() + && deleted_at < now.checked_add_days(day).unwrap(), + "Server's deleted_at time was incorrect", + ); +} + +#[actix::test] +async fn test_soft_delete_failure() { + let mut app = TestApp::spawn().await; + + let id = Uuid::new_v4(); + let mut response = app + .client + .request( + Path::DELETE(format!("{}/{}", BASE_PATH, id)), + &[Header::ContentType(ContentType::Json)], + None::, + ) + .await; + + assert_eq!( + 404, + response.status(), + "The APII did not return 404 when trying to soft delete a non-existant server", + ); + + let user = app + .database + .insert_user("testuser@email.com", "test.user", true) + .await; + let server = app.database.insert_server(user.id()).await; + + response = app + .client + .request( + Path::DELETE(format!("{}/{}", BASE_PATH, server.id())), + &[Header::ContentType(ContentType::Json)], + None::, + ) + .await; + + assert_eq!( + 200, + response.status(), + "Unexpectedly failed to soft delete test server: {}", + response.text().await.unwrap_or_default(), + ); + + response = app + .client + .request( + Path::DELETE(format!("{}/{}", BASE_PATH, server.id())), + &[Header::ContentType(ContentType::Json)], + None::, + ) + .await; + + assert_eq!( + 400, + response.status(), + "The API did not return 400 when trying to soft delete a server that has already been soft deleted", + ); + + assert_eq!( + format!("server {} has already been soft deleted", server.id()), + response.text().await.unwrap_or_default(), + "The response body did not match expected text" + ); +} + +#[actix::test] +async fn test_hard_delete_success() { + let mut app = TestApp::spawn().await; + + let user = app + .database + .insert_user("testuser@email.com", "test.user", true) + .await; + let server = app.database.insert_server(user.id()).await; + + let response = app + .client + .request( + Path::DELETE(format!("{}/{}/hard", BASE_PATH, server.id())), + &[Header::ContentType(ContentType::Json)], + None::, + ) + .await; + + assert_eq!( + 200, + response.status(), + "The API did not return 200 on valid server hard delete: {}", + response.text().await.unwrap_or_default(), + ); + + assert_err!( + app.database.get_server_by_id(server.id()).await, + "Server {} still exists in the database", + server.id(), + ); +} diff --git a/tests/api/server/mod.rs b/tests/api/server/mod.rs index b1023b0..b9d8b55 100644 --- a/tests/api/server/mod.rs +++ b/tests/api/server/mod.rs @@ -1,2 +1,3 @@ mod create; +mod delete; mod update; diff --git a/tests/api/server/update.rs b/tests/api/server/update.rs index 9146c79..e572466 100644 --- a/tests/api/server/update.rs +++ b/tests/api/server/update.rs @@ -152,7 +152,14 @@ async fn test_update_server_success() { error_case, ); - server = app.database.get_server_by_id(server.id()).await; + server = match app.database.get_server_by_id(server.id()).await { + Ok(server) => server, + Err(e) => panic!( + "failed to retrieve server {} from database: {}", + server.id(), + e + ), + }; assert_eq!( body.name(), diff --git a/tests/api/user/delete.rs b/tests/api/user/delete.rs index 0cbc3d9..5244849 100644 --- a/tests/api/user/delete.rs +++ b/tests/api/user/delete.rs @@ -2,7 +2,7 @@ use crate::utils::{ app::TestApp, http_client::{ContentType, Header, Path}, }; -use chrono::{DateTime, Days, Utc}; +use chrono::{Days, Utc}; use claim::{assert_err, assert_none, assert_ok, assert_some}; use fake::{faker::internet::en::SafeEmail, Fake}; use muttr_server::{domain::user, handlers::user::BASE_PATH}; diff --git a/tests/utils/db/server.rs b/tests/utils/db/server.rs index 681b6e7..1a408b3 100644 --- a/tests/utils/db/server.rs +++ b/tests/utils/db/server.rs @@ -28,7 +28,7 @@ impl TestDB { } } - pub async fn get_server_by_id(&mut self, id: Uuid) -> Server { - get_server_by_id(&self.db_pool, id).await.unwrap() + pub async fn get_server_by_id(&mut self, id: Uuid) -> Result { + get_server_by_id(&self.db_pool, id).await } }