Improve (and test) error handling

Non existent projects, users and such.

3 files changed, 280 insertions, 16 deletions

diff --git a/server/src/api_model.rs b/server/src/api_model.rs
index 6c614e5..3e94d6c 100644
--- a/server/src/api_model.rs
+++ b/server/src/api_model.rs
@@ -66,7 +66,7 @@ pub struct Review {
     pub archived: bool,
 }
 
-#[derive(Serialize, ToSchema)]
+#[derive(Deserialize, Serialize, ToSchema)]
 pub struct ReviewEntry {
     #[schema(example = 1000u64)]
     pub id: u64,
@@ -79,7 +79,7 @@ pub struct ReviewEntry {
     pub progress: f32,
 }
 
-#[derive(Serialize, ToSchema)]
+#[derive(Deserialize, Serialize, ToSchema)]
 pub struct Reviews {
     #[schema(example = 0u32)]
     pub offset: u32,
diff --git a/server/src/main.rs b/server/src/main.rs
index e48cfb7..d70c4e7 100644
--- a/server/src/main.rs
+++ b/server/src/main.rs
@@ -5,7 +5,7 @@ use futures::{future::TryFutureExt, stream::TryStreamExt};
 use rocket::fairing::{self, AdHoc};
 use rocket::figment::Figment;
 use rocket::http::Status;
-use rocket::response::status::{Custom, NotFound};
+use rocket::response::status::{Conflict, Custom, NotFound};
 use rocket::serde::json::Json;
 use rocket::{futures, Build, Rocket};
 use rocket_db_pools::{sqlx, Connection, Database};
@@ -192,6 +192,7 @@ async fn project(
 #[utoipa::path(
     responses(
         (status = 200, description = "Project created", body = api_model::Project),
+        (status = 409, description = "Project with id already exists"),
     ),
     security(
         ("session" = []),
@@ -203,7 +204,7 @@ async fn project_new(
     session: auth::Session,
     projectid: &str,
     data: Json<api_model::ProjectData<'_>>,
-) -> Json<api_model::Project> {
+) -> Result<Json<api_model::Project>, Conflict<&'static str>> {
     {
         let mut tx = db.begin().await.unwrap();
 
@@ -216,8 +217,8 @@ async fn project_new(
             data.main_branch.unwrap_or("main"),
         )
         .execute(&mut *tx)
-        .await
-        .unwrap();
+        .map_err(|_| Conflict("Project with id already exists"))
+        .await?;
 
         sqlx::query!(
             "INSERT INTO project_users (project, user, default_role, maintainer) VALUES (?, ?, ?, ?)",
@@ -229,7 +230,7 @@ async fn project_new(
         tx.commit().await.unwrap();
     }
 
-    get_project(&mut db, projectid).await.unwrap()
+    Ok(get_project(&mut db, projectid).await.unwrap())
 }
 
 async fn project_check_maintainer(
@@ -310,6 +311,7 @@ async fn project_update(
         (status = 200, description = "User added to project"),
         (status = 401, description = "Not maintainer of project"),
         (status = 404, description = "No such project"),
+        (status = 409, description = "User already in project"),
     ),
     security(
         ("session" = []),
@@ -336,8 +338,8 @@ async fn project_user_add(
         data.maintainer.unwrap_or(false)
     )
     .execute(&mut **db)
-    .await
-    .unwrap();
+    .map_err(|_| Custom(Status::Conflict, "User already in project"))
+    .await?;
 
     Ok("")
 }
@@ -346,7 +348,7 @@ async fn project_user_add(
     responses(
         (status = 200, description = "User updated in project"),
         (status = 401, description = "Not maintainer of project"),
-        (status = 404, description = "No such project"),
+        (status = 404, description = "No such project, no such user or user not in project"),
     ),
     security(
         ("session" = []),
@@ -383,7 +385,10 @@ async fn project_user_update(
 
         let mut query_builder: sqlx::QueryBuilder<sqlx::MySql> =
             sqlx::QueryBuilder::with_arguments(query, args);
-        query_builder.build().execute(&mut **db).await.unwrap();
+        let result = query_builder.build().execute(&mut **db).await.unwrap();
+        if result.rows_affected() == 0 {
+            return Err(Custom(Status::NotFound, "No such project or no such user"));
+        }
     }
 
     Ok("")
@@ -393,7 +398,7 @@ async fn project_user_update(
     responses(
         (status = 200, description = "User removed from project"),
         (status = 401, description = "Not maintainer of project"),
-        (status = 404, description = "No such project"),
+        (status = 404, description = "No such project, no such user or user not in project"),
     ),
     security(
         ("session" = []),
@@ -412,7 +417,7 @@ async fn project_user_del(
         project_check_maintainer(&mut db, session, projectid).await?;
     }
 
-    sqlx::query!(
+    let result = sqlx::query!(
         "DELETE FROM project_users WHERE project=? AND user=?",
         projectid,
         userid
@@ -420,6 +425,9 @@ async fn project_user_del(
     .execute(&mut **db)
     .await
     .unwrap();
+    if result.rows_affected() == 0 {
+        return Err(Custom(Status::NotFound, "No such project or no such user"));
+    }
 
     Ok("")
 }
@@ -427,6 +435,7 @@ async fn project_user_del(
 #[utoipa::path(
     responses(
         (status = 200, description = "Get all reviews for project", body = api_model::Reviews),
+        (status = 404, description = "No such project"),
     ),
     security(
         ("session" = []),
@@ -439,7 +448,7 @@ async fn reviews(
     projectid: &str,
     limit: Option<u32>,
     offset: Option<u32>,
-) -> Json<api_model::Reviews> {
+) -> Result<Json<api_model::Reviews>, NotFound<&'static str>> {
     let uw_offset = offset.unwrap_or(0);
     let uw_limit = limit.unwrap_or(10);
     let entries = sqlx::query!(
@@ -470,15 +479,29 @@ async fn reviews(
     .await
     .unwrap();
 
+    if count == 0 {
+        let projects = sqlx::query!(
+            "SELECT COUNT(id) AS count FROM projects WHERE id=?",
+            projectid
+        )
+        .fetch_one(&mut **db)
+        .map_ok(|r| r.count)
+        .await
+        .unwrap();
+        if projects == 0 {
+            return Err(NotFound("No such project"));
+        }
+    }
+
     let u32_count = u32::try_from(count).unwrap();
 
-    Json(api_model::Reviews {
+    Ok(Json(api_model::Reviews {
         offset: uw_offset,
         limit: uw_limit,
         total_count: u32_count,
         more: uw_offset + uw_limit < u32_count,
         reviews: entries,
-    })
+    }))
 }
 
 #[utoipa::path(
diff --git a/server/src/tests.rs b/server/src/tests.rs
index 72bdd78..b96a5b3 100644
--- a/server/src/tests.rs
+++ b/server/src/tests.rs
@@ -4,6 +4,7 @@ use rocket::http::{ContentType, Header, Status};
 use rocket::local::asynchronous::{Client, LocalRequest};
 use sqlx::mysql::{MySql, MySqlConnectOptions, MySqlPoolOptions};
 use sqlx::{Acquire, Executor, Pool};
+use std::fmt::Display;
 use std::sync::OnceLock;
 use stdext::function_name;
 
@@ -191,6 +192,17 @@ async fn new_project(client: &Client) -> api_model::Project {
     .await
 }
 
+async fn get_reviews<'a>(client: &Client, project_url: impl Display) -> api_model::Reviews {
+    client
+        .get(format!("{project_url}/reviews"))
+        .header(&FAKE_IP)
+        .dispatch()
+        .await
+        .into_json::<api_model::Reviews>()
+        .await
+        .unwrap()
+}
+
 #[rocket::async_test]
 async fn test_not_logged_in_status() {
     let client = async_client_with_private_database(function_name!().to_string()).await;
@@ -305,6 +317,31 @@ async fn test_project_new() {
 }
 
 #[rocket::async_test]
+async fn test_project_new_duplicate() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    new_project(&client).await;
+
+    let duplicate_project = client
+        .post("/api/v1/project/test/new")
+        .json(&api_model::ProjectData {
+            title: Some("foo"),
+            description: Some("bar"),
+            remote: Some("fum"),
+            main_branch: Some("zod"),
+        })
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(duplicate_project.status(), Status::Conflict);
+
+    let projects = get_projects(&client).await;
+    assert_eq!(projects.total_count, 1);
+}
+
+#[rocket::async_test]
 async fn test_project_update() {
     let client = async_client_with_private_database(function_name!().to_string()).await;
 
@@ -343,6 +380,26 @@ async fn test_project_update() {
 }
 
 #[rocket::async_test]
+async fn test_project_update_unknown() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let update = client
+        .post("/api/v1/project/does_not_exist")
+        .json(&api_model::ProjectData {
+            title: Some("foo"),
+            description: Some("bar"),
+            remote: Some("fum"),
+            main_branch: Some("zod"),
+        })
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(update.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
 async fn test_project_new_user() {
     let client = async_client_with_private_database(function_name!().to_string()).await;
 
@@ -378,6 +435,41 @@ async fn test_project_new_user() {
 }
 
 #[rocket::async_test]
+async fn test_project_new_user_duplicate() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let project = new_project(&client).await;
+    let project_url = format!("/api/v1/project/{}", project.id);
+
+    let users = get_users(&client).await;
+    let other = users.users.iter().find(|u| u.id == "other").unwrap();
+
+    let new = client
+        .post(format!("{project_url}/user/{}/new", other.id))
+        .json(&api_model::ProjectUserEntryData {
+            default_role: Some(api_model::UserReviewRole::Watcher),
+            maintainer: Some(true),
+        })
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(new.status(), Status::Ok);
+
+    let duplicate = client
+        .post(format!("{project_url}/user/{}/new", other.id))
+        .json(&api_model::ProjectUserEntryData {
+            default_role: Some(api_model::UserReviewRole::Reviewer),
+            maintainer: Some(false),
+        })
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(duplicate.status(), Status::Conflict);
+}
+
+#[rocket::async_test]
 async fn test_project_change_user() {
     let client = async_client_with_private_database(function_name!().to_string()).await;
 
@@ -425,6 +517,72 @@ async fn test_project_change_user() {
 }
 
 #[rocket::async_test]
+async fn test_project_change_user_unknown_project() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let users = get_users(&client).await;
+    let other = users.users.iter().find(|u| u.id == "other").unwrap();
+
+    let update = client
+        .post(format!("/api/v1/project/does_not_exist/user/{}", other.id))
+        .json(&api_model::ProjectUserEntryData {
+            default_role: Some(api_model::UserReviewRole::Watcher),
+            maintainer: Some(true),
+        })
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(update.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
+async fn test_project_change_user_unknown_user() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let project = new_project(&client).await;
+    let project_url = format!("/api/v1/project/{}", project.id);
+
+    let update = client
+        .post(format!("{project_url}/user/does_not_exist"))
+        .json(&api_model::ProjectUserEntryData {
+            default_role: Some(api_model::UserReviewRole::Watcher),
+            maintainer: Some(true),
+        })
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(update.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
+async fn test_project_change_user_not_in_project() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let project = new_project(&client).await;
+    let project_url = format!("/api/v1/project/{}", project.id);
+
+    let users = get_users(&client).await;
+    let other = users.users.iter().find(|u| u.id == "other").unwrap();
+
+    let update = client
+        .post(format!("{project_url}/user/{}", other.id))
+        .json(&api_model::ProjectUserEntryData {
+            default_role: None,
+            maintainer: Some(false),
+        })
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(update.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
 async fn test_project_check_maintainer() {
     let client = async_client_with_private_database(function_name!().to_string()).await;
 
@@ -572,3 +730,86 @@ async fn test_project_delete_user() {
     let other_entry = updated_project.users.get(0).unwrap();
     assert_eq!(other_entry.user, *other);
 }
+
+#[rocket::async_test]
+async fn test_project_delete_user_unknown_project() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let users = get_users(&client).await;
+    let user = users.users.iter().find(|u| u.id == "user").unwrap();
+
+    let delete = client
+        .delete(format!("/api/v1/project/does_not_exist/user/{}", user.id))
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(delete.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
+async fn test_project_delete_user_unknown_user() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let project = new_project(&client).await;
+    let project_url = format!("/api/v1/project/{}", project.id);
+
+    let delete = client
+        .delete(format!("{project_url}/user/does_not_exist"))
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(delete.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
+async fn test_project_delete_user_not_in_project() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let project = new_project(&client).await;
+    let project_url = format!("/api/v1/project/{}", project.id);
+
+    let users = get_users(&client).await;
+    let other = users.users.iter().find(|u| u.id == "other").unwrap();
+
+    let delete = client
+        .delete(format!("{project_url}/user/{}", other.id))
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(delete.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
+async fn test_project_reviews_empty_project() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let project = new_project(&client).await;
+    let project_url = format!("/api/v1/project/{}", project.id);
+
+    let reviews = get_reviews(&client, project_url).await;
+    assert_eq!(reviews.total_count, 0);
+    assert_eq!(reviews.more, false);
+    assert_eq!(reviews.reviews.len(), 0);
+}
+
+#[rocket::async_test]
+async fn test_project_reviews_unknown_project() {
+    let client = async_client_with_private_database(function_name!().to_string()).await;
+
+    login(&client).await;
+
+    let reviews = client
+        .get("/api/v1/project/does_not_exist/reviews")
+        .header(&FAKE_IP)
+        .dispatch()
+        .await;
+    assert_eq!(reviews.status(), Status::NotFound);
+}