From b7afe5ecfcae0a4107fe61b2c6129a507616ee49 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 20 Dec 2022 12:51:44 +0100 Subject: [PATCH] improve error message for already existing items --- backend/sqlx-data.json | 209 +++++++++++++++++++++++++ backend/windmill-api/src/flows.rs | 22 +++ backend/windmill-api/src/folders.rs | 23 +++ backend/windmill-api/src/groups.rs | 24 +++ backend/windmill-api/src/resources.rs | 73 +++++++-- backend/windmill-api/src/schedule.rs | 23 +++ backend/windmill-api/src/scripts.rs | 1 + backend/windmill-api/src/variables.rs | 46 ++++-- backend/windmill-api/src/workspaces.rs | 17 +- 9 files changed, 416 insertions(+), 22 deletions(-) diff --git a/backend/sqlx-data.json b/backend/sqlx-data.json index 7af294595e..295ba6b6ee 100644 --- a/backend/sqlx-data.json +++ b/backend/sqlx-data.json @@ -226,6 +226,27 @@ }, "query": "SELECT name FROM folder WHERE workspace_id = $1 ORDER BY name desc LIMIT $2 OFFSET $3" }, + "0c3b687d97cb0bb2dfec369c013e7570ee9c3f473c6e92b1161ed71527385348": { + "describe": { + "columns": [ + { + "name": "exists", + "ordinal": 0, + "type_info": "Bool" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT EXISTS(SELECT 1 FROM flow WHERE path = $1 AND workspace_id = $2)" + }, "0c5faa1e3233ca15240f80c9ba805f839d9f1f7450d1ea1f2b8e9faf8533892d": { "describe": { "columns": [], @@ -471,6 +492,27 @@ }, "query": "SELECT dependency_job FROM flow WHERE path = $1 AND workspace_id = $2" }, + "1a8d9f265302f0f9175e02cc4b3f26fa0e9cbbaab89299e940eb4e94d8e62cf1": { + "describe": { + "columns": [ + { + "name": "count", + "ordinal": 0, + "type_info": "Int8" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT count(path) FROM script WHERE path LIKE 'f/' || $1 || '%' AND archived IS false AND workspace_id = $2" + }, "1b31847d6187d6969deac5aa7b2feb169ef963449ac2d3ea06e1ed785f6d42e7": { "describe": { "columns": [ @@ -2373,6 +2415,27 @@ }, "query": "SELECT * FROM resource_type WHERE workspace_id = $1" }, + "7b23055f7d7ede8943637e54a76c43583ef80e475f527c6bbb660329e4f1ac7a": { + "describe": { + "columns": [ + { + "name": "exists", + "ordinal": 0, + "type_info": "Bool" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT EXISTS(SELECT 1 FROM folder WHERE name = $1 AND workspace_id = $2)" + }, "7eeac533a0d63f4e3af9d3e3123b0a73f44543e618e29e4c6a6d573852339933": { "describe": { "columns": [ @@ -2814,6 +2877,27 @@ }, "query": "SELECT * FROM schedule WHERE workspace_id = $1 ORDER BY edited_at desc LIMIT $2 OFFSET $3" }, + "8cc4fa257cb8053d27eb7a555d6e1b993e4a74b063eac632df8da24f6b87130e": { + "describe": { + "columns": [ + { + "name": "count", + "ordinal": 0, + "type_info": "Int8" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT count(path) FROM resource WHERE path LIKE 'f/' || $1 || '%' AND workspace_id = $2" + }, "8cea673a5b17bc7cf671b539c2c1eb34731c77aaf42dc12e45472eeb134887d0": { "describe": { "columns": [ @@ -2868,6 +2952,27 @@ }, "query": "SELECT value FROM flow WHERE path = $1 AND (workspace_id = $2 OR workspace_id = 'starter')" }, + "8d678edbc19a195ef4c26c2deb4bb2bd6dbe9253356f1cd617f5522d8d65c238": { + "describe": { + "columns": [ + { + "name": "count", + "ordinal": 0, + "type_info": "Int8" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT count(path) FROM flow WHERE path LIKE 'f/' || $1 || '%' AND archived IS false AND workspace_id = $2" + }, "8da6ff304d199401ee8ee9a3de1d51477c04f9abc1b6255d84c5b1fc58267c59": { "describe": { "columns": [], @@ -3443,6 +3548,27 @@ }, "query": "\n UPDATE queue\n SET flow_status = JSONB_SET(flow_status, ARRAY['failure_module'], $1)\n WHERE id = $2\n " }, + "a7abe946c8d0653169e292560eb7a5aa50a9c0f590444480ac642afcb83bbffb": { + "describe": { + "columns": [ + { + "name": "exists", + "ordinal": 0, + "type_info": "Bool" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT EXISTS(SELECT 1 FROM group_ WHERE name = $1 AND workspace_id = $2)" + }, "a7b028e832396ee4d1ad6dfd44ba6134344f3eb37dbf0254154eba31b9cc2ed3": { "describe": { "columns": [ @@ -3741,6 +3867,27 @@ }, "query": "SELECT * from workspace_invite WHERE workspace_id = $1" }, + "b3d4787ad09757725a721228e0f3deaf3ee851ba23f866da50162e362764d6a6": { + "describe": { + "columns": [ + { + "name": "count", + "ordinal": 0, + "type_info": "Int8" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT count(path) FROM schedule WHERE path LIKE 'f/' || $1 || '%' AND workspace_id = $2" + }, "b5c9891b5bf3d581e62f8835aaa25b8158fe3cefc849af0ba312a45cf22721ca": { "describe": { "columns": [ @@ -4442,6 +4589,27 @@ }, "query": "SELECT app.id, app.path, app.summary, app.versions, app.policy,\n app.extra_perms, app_version.value, \n app_version.created_at, app_version.created_by from app, app_version \n WHERE app.path = $1 AND app.workspace_id = $2 AND app_version.id = app.versions[array_upper(app.versions, 1)]" }, + "d12d151995da2e8357307dd99e3f7b59192832a566d56b56097d15f713430705": { + "describe": { + "columns": [ + { + "name": "count", + "ordinal": 0, + "type_info": "Int8" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT count(path) FROM app WHERE path LIKE 'f/' || $1 || '%' AND workspace_id = $2" + }, "d47bff7d6b54cd6da8bb330f7321c37af5dcbd76f9acad73b5ba1b8a4afb5091": { "describe": { "columns": [ @@ -4545,6 +4713,27 @@ }, "query": "UPDATE script SET lock = $1 WHERE hash = $2 AND workspace_id = $3" }, + "d6acfbc2e4ef1a4f4252cf7566d91fc4d80a4bb0e383495efa82c697b17d2bf8": { + "describe": { + "columns": [ + { + "name": "count", + "ordinal": 0, + "type_info": "Int8" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + } + }, + "query": "SELECT count(path) FROM variable WHERE path LIKE 'f/' || $1 || '%' AND workspace_id = $2" + }, "d768bbc46f8a9c4289b918c88ca50aa180b29bbe931d948f6e61976f71b7cdb9": { "describe": { "columns": [ @@ -4659,6 +4848,26 @@ }, "query": "UPDATE app SET versions = array_append(versions, $1) WHERE path = $2 AND workspace_id = $3" }, + "dd7940ec390357b268d616e1880516ecc08d506db2109efcce840f096d7a594e": { + "describe": { + "columns": [ + { + "name": "exists", + "ordinal": 0, + "type_info": "Bool" + } + ], + "nullable": [ + null + ], + "parameters": { + "Left": [ + "Text" + ] + } + }, + "query": "SELECT EXISTS(SELECT 1 FROM workspace WHERE id = $1)" + }, "de4c8f42ff846599d6b2b26615a34f0bcbad974edc5f00fb0491475017eebaeb": { "describe": { "columns": [], diff --git a/backend/windmill-api/src/flows.rs b/backend/windmill-api/src/flows.rs index cf940cb18d..6a525f2b7d 100644 --- a/backend/windmill-api/src/flows.rs +++ b/backend/windmill-api/src/flows.rs @@ -140,6 +140,25 @@ pub async fn get_hub_flow_by_id( Ok(Json(value)) } +async fn check_path_conflict<'c>( + tx: &mut Transaction<'c, Postgres>, + w_id: &str, + path: &str, +) -> Result<()> { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM flow WHERE path = $1 AND workspace_id = $2)", + path, + w_id + ) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(Error::BadRequest(format!("Flow {} already exists", path))); + } + return Ok(()); +} + async fn create_flow( authed: Authed, Extension(user_db): Extension, @@ -149,6 +168,7 @@ async fn create_flow( // cron::Schedule::from_str(&ns.schedule).map_err(|e| error::Error::BadRequest(e.to_string()))?; let mut tx = user_db.clone().begin(&authed).await?; + check_path_conflict(&mut tx, &w_id, &nf.path).await?; check_schedule_conflict(&mut tx, &w_id, &nf.path).await?; sqlx::query!( @@ -274,6 +294,8 @@ async fn update_flow( .await?; if nf.path != flow_path { + check_schedule_conflict(&mut tx, &w_id, &nf.path).await?; + if !authed.is_admin { require_owner_of_path(&w_id, &authed.username, &flow_path, &db).await?; } diff --git a/backend/windmill-api/src/folders.rs b/backend/windmill-api/src/folders.rs index b0395a45fc..761c10180e 100644 --- a/backend/windmill-api/src/folders.rs +++ b/backend/windmill-api/src/folders.rs @@ -104,6 +104,28 @@ async fn list_foldernames( Ok(Json(rows)) } +async fn check_name_conflict<'c>( + tx: &mut Transaction<'c, Postgres>, + w_id: &str, + name: &str, +) -> Result<()> { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM folder WHERE name = $1 AND workspace_id = $2)", + name, + w_id + ) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(windmill_common::error::Error::BadRequest(format!( + "Folder {} already exists", + name + ))); + } + return Ok(()); +} + async fn create_folder( authed: Authed, Extension(user_db): Extension, @@ -112,6 +134,7 @@ async fn create_folder( ) -> Result { let mut tx = user_db.begin(&authed).await?; + check_name_conflict(&mut tx, &w_id, &ng.name).await?; let owner = owner_to_token_owner(&authed.username, false); sqlx::query_as!( Folder, diff --git a/backend/windmill-api/src/groups.rs b/backend/windmill-api/src/groups.rs index aa9b680f2c..3d4310b180 100644 --- a/backend/windmill-api/src/groups.rs +++ b/backend/windmill-api/src/groups.rs @@ -115,6 +115,28 @@ async fn list_group_names( Ok(Json(rows)) } +async fn check_name_conflict<'c>( + tx: &mut Transaction<'c, Postgres>, + w_id: &str, + name: &str, +) -> Result<()> { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM group_ WHERE name = $1 AND workspace_id = $2)", + name, + w_id + ) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(windmill_common::error::Error::BadRequest(format!( + "Group {} already exists", + name + ))); + } + return Ok(()); +} + async fn create_group( authed: Authed, Extension(user_db): Extension, @@ -123,6 +145,8 @@ async fn create_group( ) -> Result { let mut tx = user_db.begin(&authed).await?; + check_name_conflict(&mut tx, &w_id, &ng.name).await?; + sqlx::query_as!( Group, "INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES ($1, $2, $3, $4)", diff --git a/backend/windmill-api/src/resources.rs b/backend/windmill-api/src/resources.rs index ab35e2c3a4..1a737138ce 100644 --- a/backend/windmill-api/src/resources.rs +++ b/backend/windmill-api/src/resources.rs @@ -18,7 +18,7 @@ use axum::{ use hyper::StatusCode; use serde::{Deserialize, Serialize}; use sql_builder::{bind::Bind, SqlBuilder}; -use sqlx::FromRow; +use sqlx::{FromRow, Postgres, Transaction}; use windmill_audit::{audit_log, ActionKind}; use windmill_common::{ error::{Error, JsonResult, Result}, @@ -237,6 +237,28 @@ async fn get_resource_value( Ok(Json(value)) } +async fn check_path_conflict<'c>( + tx: &mut Transaction<'c, Postgres>, + w_id: &str, + path: &str, +) -> Result<()> { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM resource WHERE path = $1 AND workspace_id = $2)", + path, + w_id + ) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(Error::BadRequest(format!( + "Resource {} already exists", + path + ))); + } + return Ok(()); +} + async fn create_resource( authed: Authed, Extension(user_db): Extension, @@ -245,6 +267,7 @@ async fn create_resource( ) -> Result<(StatusCode, String)> { let mut tx = user_db.begin(&authed).await?; + check_path_conflict(&mut tx, &w_id, &resource.path).await?; sqlx::query!( "INSERT INTO resource (workspace_id, path, value, description, resource_type) @@ -347,17 +370,21 @@ async fn update_resource( let npath = not_found_if_none(npath_o, "Resource", path)?; if let Some(npath) = ns.path { - if !authed.is_admin { - require_owner_of_path(&w_id, &authed.username, &path, &db).await?; + if npath != path { + check_path_conflict(&mut tx, &w_id, &npath).await?; + + if !authed.is_admin { + require_owner_of_path(&w_id, &authed.username, &path, &db).await?; + } + sqlx::query!( + "UPDATE variable SET path = $1 WHERE path = $2 AND workspace_id = $3", + npath, + path, + w_id + ) + .execute(&mut tx) + .await?; } - sqlx::query!( - "UPDATE variable SET path = $1 WHERE path = $2 AND workspace_id = $3", - npath, - path, - w_id - ) - .execute(&mut tx) - .await?; } audit_log( @@ -452,6 +479,8 @@ async fn create_resource_type( ) -> Result<(StatusCode, String)> { let mut tx = user_db.begin(&authed).await?; + check_rt_path_conflict(&mut tx, &w_id, &resource_type.name).await?; + sqlx::query!( "INSERT INTO resource_type (workspace_id, name, schema, description) @@ -481,6 +510,28 @@ async fn create_resource_type( )) } +async fn check_rt_path_conflict<'c>( + tx: &mut Transaction<'c, Postgres>, + w_id: &str, + name: &str, +) -> Result<()> { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM resource_type WHERE name = $1 AND workspace_id = $2)", + name, + w_id + ) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(Error::BadRequest(format!( + "Resource type {} already exists", + name + ))); + } + return Ok(()); +} + async fn delete_resource_type( authed: Authed, Extension(user_db): Extension, diff --git a/backend/windmill-api/src/schedule.rs b/backend/windmill-api/src/schedule.rs index 262c4aa8ca..99c21f1acf 100644 --- a/backend/windmill-api/src/schedule.rs +++ b/backend/windmill-api/src/schedule.rs @@ -54,6 +54,28 @@ pub struct NewSchedule { pub enabled: Option, } +async fn check_path_conflict<'c>( + tx: &mut Transaction<'c, Postgres>, + w_id: &str, + path: &str, +) -> Result<()> { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM schedule WHERE path = $1 AND workspace_id = $2)", + path, + w_id + ) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(Error::BadRequest(format!( + "Schedule {} already exists", + path + ))); + } + return Ok(()); +} + async fn create_schedule( authed: Authed, Extension(user_db): Extension, @@ -62,6 +84,7 @@ async fn create_schedule( ) -> Result { let mut tx = user_db.begin(&authed).await?; cron::Schedule::from_str(&ns.schedule).map_err(|e| Error::BadRequest(e.to_string()))?; + check_path_conflict(&mut tx, &w_id, &ns.path).await?; check_flow_conflict(&mut tx, &w_id, &ns.path, ns.is_flow, &ns.script_path).await?; let schedule = sqlx::query_as!( diff --git a/backend/windmill-api/src/scripts.rs b/backend/windmill-api/src/scripts.rs index 391a74f497..2522da6676 100644 --- a/backend/windmill-api/src/scripts.rs +++ b/backend/windmill-api/src/scripts.rs @@ -179,6 +179,7 @@ fn hash_script(ns: &NewScript) -> i64 { ns.hash(&mut dh); dh.finish() as i64 } + async fn create_script( authed: Authed, Extension(user_db): Extension, diff --git a/backend/windmill-api/src/variables.rs b/backend/windmill-api/src/variables.rs index 54a93ccf94..c4de9defeb 100644 --- a/backend/windmill-api/src/variables.rs +++ b/backend/windmill-api/src/variables.rs @@ -199,6 +199,28 @@ async fn exists_variable( Ok(Json(exists)) } +async fn check_path_conflict<'c>( + tx: &mut Transaction<'c, Postgres>, + w_id: &str, + path: &str, +) -> Result<()> { + let exists = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM variable WHERE path = $1 AND workspace_id = $2)", + path, + w_id + ) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(Error::BadRequest(format!( + "Variable {} already exists", + path + ))); + } + return Ok(()); +} + async fn create_variable( authed: Authed, Extension(user_db): Extension, @@ -207,6 +229,7 @@ async fn create_variable( ) -> Result<(StatusCode, String)> { let mut tx = user_db.begin(&authed).await?; + check_path_conflict(&mut tx, &w_id, &variable.path).await?; let value = if variable.is_secret { let mc = build_crypt(&mut tx, &w_id).await?; encrypt(&mc, &variable.value) @@ -352,17 +375,20 @@ async fn update_variable( let npath_o: Option = sqlx::query_scalar(&sql).fetch_optional(&mut tx).await?; if let Some(npath) = ns.path { - if !authed.is_admin { - require_owner_of_path(&w_id, &authed.username, &path, &db).await?; + if npath != path { + check_path_conflict(&mut tx, &w_id, &npath).await?; + if !authed.is_admin { + require_owner_of_path(&w_id, &authed.username, &path, &db).await?; + } + sqlx::query!( + "UPDATE resource SET path = $1 WHERE path = $2 AND workspace_id = $3", + npath, + path, + w_id + ) + .execute(&mut tx) + .await?; } - sqlx::query!( - "UPDATE resource SET path = $1 WHERE path = $2 AND workspace_id = $3", - npath, - path, - w_id - ) - .execute(&mut tx) - .await?; } let npath = not_found_if_none(npath_o, "Variable", path)?; diff --git a/backend/windmill-api/src/workspaces.rs b/backend/windmill-api/src/workspaces.rs index b8f35c5fac..680dc477c3 100644 --- a/backend/windmill-api/src/workspaces.rs +++ b/backend/windmill-api/src/workspaces.rs @@ -31,7 +31,7 @@ use windmill_common::{ use hyper::{header, StatusCode}; use serde::{Deserialize, Serialize}; -use sqlx::FromRow; +use sqlx::{FromRow, Postgres, Transaction}; use tempfile::TempDir; use tokio::fs::File; use tokio_util::io::ReaderStream; @@ -395,6 +395,20 @@ async fn user_workspaces( Ok(Json(WorkspaceList { email, workspaces })) } +async fn check_name_conflict<'c>(tx: &mut Transaction<'c, Postgres>, w_id: &str) -> Result<()> { + let exists = sqlx::query_scalar!("SELECT EXISTS(SELECT 1 FROM workspace WHERE id = $1)", w_id) + .fetch_one(tx) + .await? + .unwrap_or(false); + if exists { + return Err(windmill_common::error::Error::BadRequest(format!( + "Workspace {} already exists", + w_id + ))); + } + return Ok(()); +} + async fn create_workspace( authed: Authed, Extension(db): Extension, @@ -404,6 +418,7 @@ async fn create_workspace( return Err(Error::BadRequest("bot is a reserved username".to_string())); } let mut tx = db.begin().await?; + check_name_conflict(&mut tx, &nw.id).await?; sqlx::query!( "INSERT INTO workspace (id, name, owner)