Return 409 conflict when POST /maps with taken name. close #758

Author: horenso

backend/src/model/entity/map_impl.rs

@@ -1,11 +1,11 @@
 //! Contains the implementation of [`Map`].
 
-use diesel::dsl::sql;
+use diesel::dsl::{exists, sql};
 use diesel::pg::Pg;
 use diesel::sql_types::Float;
 use diesel::{
-    debug_query, BoolExpressionMethods, ExpressionMethods, PgTextExpressionMethods, QueryDsl,
-    QueryResult,
+    debug_query, select, BoolExpressionMethods, ExpressionMethods, PgTextExpressionMethods,
+    QueryDsl, QueryResult,
 };
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
 use log::debug;
@@ -87,6 +87,16 @@ impl Map {
         query.first::<Self>(conn).await.map(Into::into)
     }
 
+    /// Checks if a map with this name already exists in the database.
+    ///
+    /// # Errors
+    /// * Unknown, diesel doesn't say why it might error.
+    pub async fn is_name_taken(map_name: &str, conn: &mut AsyncPgConnection) -> QueryResult<bool> {
+        let query = select(exists(maps::table.filter(name.eq(map_name))));
+        debug!("{}", debug_query::<Pg, _>(&query));
+        query.get_result(conn).await
+    }
+
     /// Create a new map in the database.
     ///
     /// # Errors

backend/src/service/map.rs

@@ -58,6 +58,13 @@ pub async fn create(
 ) -> Result<MapDto, ServiceError> {
     let mut conn = app_data.pool.get().await?;
 
+    if Map::is_name_taken(&new_map.name, &mut conn).await? {
+        return Err(ServiceError::new(
+            StatusCode::CONFLICT,
+            "Map name already taken".to_owned(),
+        ));
+    }
+
     let geometry_validation_result = is_valid_map_geometry(&new_map.geometry);
     if let Some(error) = geometry_validation_result {
         return Err(error);

backend/src/test/map.rs

@@ -1,6 +1,7 @@
 //! Tests for [`crate::controller::map`].
 
 use crate::model::dto::UpdateMapGeometryDto;
+use crate::test::util::data;
 use crate::test::util::dummy_map_polygons::small_rectangle;
 use crate::{
     model::{
@@ -165,6 +166,52 @@ async fn test_can_create_map() {
     assert_eq!(resp.status(), StatusCode::OK);
 }
 
+#[actix_rt::test]
+async fn test_conflict_on_create_map_with_taken_name() {
+    let taken_name = "taken name";
+    let pool = init_test_database(|conn| {
+        async {
+            diesel::insert_into(crate::schema::maps::table)
+                .values(data::TestInsertableMap {
+                    name: taken_name.to_owned(),
+                    ..Default::default()
+                })
+                .execute(conn)
+                .await?;
+            Ok(())
+        }
+        .scope_boxed()
+    })
+    .await;
+    let (token, app) = init_test_app(pool.clone()).await;
+
+    let map_update = NewMapDto {
+        name: taken_name.to_owned(),
+        creation_date: NaiveDate::from_ymd_opt(2023, 5, 8).expect("Could not parse date!"),
+        deletion_date: None,
+        last_visit: None,
+        is_inactive: false,
+        zoom_factor: 100,
+        honors: 0,
+        visits: 0,
+        harvested: 0,
+        privacy: PrivacyOption::Public,
+        description: None,
+        location: None,
+        geometry: tall_rectangle(),
+    };
+
+    let resp = test::TestRequest::post()
+        .uri("/api/maps")
+        .insert_header((header::AUTHORIZATION, token))
+        .set_json(map_update)
+        .send_request(&app)
+        .await;
+
+    // Expect conflict since the map name is already present in the database
+    assert_eq!(resp.status(), StatusCode::CONFLICT);
+}
+
 #[actix_rt::test]
 async fn test_update_fails_for_not_owner() {
     let pool = init_test_database(|conn| {

doc/changelog.md

@@ -23,7 +23,7 @@ Syntax: `- short text describing the change _(Your Name)_`
 - _()_
 - Add points in middle of bezier polygon _(Daniel Steinkogler)_
 - _()_
-- _()_
+- Return 409 Conflict when creating a map with taken name _(Jannis Adamek)_
 - _()_
 - _()_
 - _()_