From fdc889f73e22b53f6e4e89dd239378a9b0c4bb79 Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Mon, 28 Oct 2024 17:50:37 +0800 Subject: [PATCH 1/3] feat: handle duplicated publish names for a workspace --- libs/app-error/src/lib.rs | 8 +++++ libs/database/src/workspace.rs | 46 ++++++++++++++++++++++++++++ src/biz/workspace/publish.rs | 55 ++++++++++++++++++++++++++++++++++ tests/workspace/publish.rs | 35 ++++++++++++++++++++++ 4 files changed, 144 insertions(+) diff --git a/libs/app-error/src/lib.rs b/libs/app-error/src/lib.rs index 48782e26..c844e7be 100644 --- a/libs/app-error/src/lib.rs +++ b/libs/app-error/src/lib.rs @@ -155,6 +155,12 @@ pub enum AppError { #[error("There is existing access request for workspace {workspace_id} and view {view_id}")] AccessRequestAlreadyExists { workspace_id: Uuid, view_id: Uuid }, + + #[error("There is existing published view for workspace {workspace_id} with publish_name {publish_name}")] + PublishNameAlreadyExists { + workspace_id: Uuid, + publish_name: String, + }, } impl AppError { @@ -225,6 +231,7 @@ impl AppError { AppError::MissingView(_) => ErrorCode::MissingView, AppError::AccessRequestAlreadyExists { .. } => ErrorCode::AccessRequestAlreadyExists, AppError::TooManyImportTask(_) => ErrorCode::TooManyImportTask, + AppError::PublishNameAlreadyExists { .. } => ErrorCode::PublishNameAlreadyExists, } } } @@ -360,6 +367,7 @@ pub enum ErrorCode { CustomNamespaceTooShort = 1047, CustomNamespaceTooLong = 1048, CustomNamespaceReserved = 1049, + PublishNameAlreadyExists = 1050, } impl ErrorCode { diff --git a/libs/database/src/workspace.rs b/libs/database/src/workspace.rs index a9037c1b..cfeb70a6 100644 --- a/libs/database/src/workspace.rs +++ b/libs/database/src/workspace.rs @@ -1599,3 +1599,49 @@ pub async fn update_import_task_metadata( Ok(()) } + +#[inline] +pub async fn select_publish_name_exists( + pg_pool: &PgPool, + workspace_uuid: &Uuid, + publish_name: &str, +) -> Result { + let exists = sqlx::query_scalar!( + r#" + SELECT EXISTS( + SELECT 1 + FROM af_published_collab + WHERE workspace_id = $1 + AND publish_name = $2 + ) + "#, + workspace_uuid, + publish_name + ) + .fetch_one(pg_pool) + .await?; + + Ok(exists.unwrap_or(false)) +} + +#[inline] +pub async fn select_view_id_from_publish_name( + pg_pool: &PgPool, + workspace_uuid: &Uuid, + publish_name: &str, +) -> Result, AppError> { + let res = sqlx::query_scalar!( + r#" + SELECT view_id + FROM af_published_collab + WHERE workspace_id = $1 + AND publish_name = $2 + "#, + workspace_uuid, + publish_name + ) + .fetch_optional(pg_pool) + .await?; + + Ok(res) +} diff --git a/src/biz/workspace/publish.rs b/src/biz/workspace/publish.rs index f8c6489e..69466452 100644 --- a/src/biz/workspace/publish.rs +++ b/src/biz/workspace/publish.rs @@ -6,6 +6,7 @@ use database::{ select_default_published_view_id_for_namespace, update_published_collabs, update_workspace_default_publish_view, update_workspace_default_publish_view_set_null, }, + workspace::{select_publish_name_exists, select_view_id_from_publish_name}, }; use database_entity::dto::PatchPublishedCollab; use std::sync::Arc; @@ -291,6 +292,13 @@ impl PublishedCollabStore for PublishedCollabPostgresStore { ) -> Result<(), AppError> { for publish_item in &publish_items { check_collab_publish_name(publish_item.meta.publish_name.as_str())?; + check_view_id_publish_name_conflict( + &self.pg_pool, + workspace_id, + &publish_item.meta.view_id, + publish_item.meta.publish_name.as_str(), + ) + .await?; } let publish_items_batch_size = publish_items.len() as i64; let result = @@ -415,6 +423,14 @@ impl PublishedCollabStore for PublishedCollabS3StoreWithPostgresFallback { let mut handles: Vec> = vec![]; for publish_item in &publish_items { check_collab_publish_name(publish_item.meta.publish_name.as_str())?; + check_view_id_publish_name_conflict( + &self.pg_pool, + workspace_id, + &publish_item.meta.view_id, + publish_item.meta.publish_name.as_str(), + ) + .await?; + let object_key = get_collab_s3_key(workspace_id, &publish_item.meta.view_id); let data = publish_item.data.clone(); let bucket_client = self.bucket_client.clone(); @@ -578,6 +594,7 @@ async fn patch_collabs( for patch in patches { if let Some(new_publish_name) = patch.publish_name.as_deref() { check_collab_publish_name(new_publish_name)?; + check_publish_name_already_exists(pg_pool, workspace_id, new_publish_name).await?; } } check_workspace_owner_or_publisher(pg_pool, user_uuid, workspace_id, &view_ids).await?; @@ -587,3 +604,41 @@ async fn patch_collabs( txn.commit().await?; Ok(()) } + +/// Checks if the `publish_name` already exists for the workspace +async fn check_publish_name_already_exists( + pg_pool: &PgPool, + workspace_id: &Uuid, + publish_name: &str, +) -> Result<(), AppError> { + let publish_name_exists = select_publish_name_exists(pg_pool, workspace_id, publish_name).await?; + if publish_name_exists { + return Err(AppError::PublishNameAlreadyExists { + workspace_id: *workspace_id, + publish_name: publish_name.to_string(), + }); + } + Ok(()) +} + +/// Check if the `publish_name` already exists on another view +async fn check_view_id_publish_name_conflict( + pg_pool: &PgPool, + workspace_id: &Uuid, + view_id: &Uuid, + publish_name: &str, +) -> Result<(), AppError> { + match select_view_id_from_publish_name(pg_pool, workspace_id, publish_name).await? { + Some(published_view_id) => { + if published_view_id != *view_id { + Err(AppError::PublishNameAlreadyExists { + workspace_id: *workspace_id, + publish_name: publish_name.to_string(), + }) + } else { + Ok(()) + } + }, + None => Ok(()), + } +} diff --git a/tests/workspace/publish.rs b/tests/workspace/publish.rs index 4a955cb9..952b6f7a 100644 --- a/tests/workspace/publish.rs +++ b/tests/workspace/publish.rs @@ -95,6 +95,7 @@ async fn test_publish_doc() { let publish_name_2 = "publish-name-2"; let view_id_2 = uuid::Uuid::new_v4(); + // User publishes two collabs c.publish_collabs::( &workspace_id, vec![ @@ -123,6 +124,25 @@ async fn test_publish_doc() { .await .unwrap(); + // User cannot publish another view_id with the same publish name + let err = c + .publish_collabs::( + &workspace_id, + vec![PublishCollabItem { + meta: PublishCollabMetadata { + view_id: uuid::Uuid::new_v4(), + publish_name: publish_name_1.to_string(), + metadata: MyCustomMetadata { + title: "some_other_title".to_string(), + }, + }, + data: "some_other_yrs_data".as_bytes(), + }], + ) + .await + .unwrap_err(); + assert_eq!(err.code, ErrorCode::PublishNameAlreadyExists, "{:?}", err); + { // Check that the published collabs are listed let published_view_infos = c.list_published_views(&workspace_id).await.unwrap(); @@ -263,6 +283,21 @@ async fn test_publish_doc() { } { + // User cannot change `publish_name` if the `publish_name` already exists + // for the same workspace + let err = c + .patch_published_collabs( + &workspace_id, + &[PatchPublishedCollab { + view_id: view_id_1, + // publish_name_2 already exists + publish_name: Some(publish_name_2.to_string()), + }], + ) + .await + .unwrap_err(); + assert_eq!(err.code, ErrorCode::PublishNameAlreadyExists, "{:?}", err); + let new_publish_name_1 = "new-publish-name-1".to_string(); // User change publish name From d9fbb20869c54069566bd1b9d1fe0c18a9e40310 Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Mon, 28 Oct 2024 18:22:59 +0800 Subject: [PATCH 2/3] chore: cargo sqlx --- ...e49e1a78fa54537cb8826a20bef2769d04dd1.json | 23 +++++++++++++++++++ ...bf886f52f681c44344a814780b01fc06e4c8f.json | 23 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 .sqlx/query-12dcf313d0e4c0c0da2569f3326e49e1a78fa54537cb8826a20bef2769d04dd1.json create mode 100644 .sqlx/query-2571550f7e2dd81103960741fecbf886f52f681c44344a814780b01fc06e4c8f.json diff --git a/.sqlx/query-12dcf313d0e4c0c0da2569f3326e49e1a78fa54537cb8826a20bef2769d04dd1.json b/.sqlx/query-12dcf313d0e4c0c0da2569f3326e49e1a78fa54537cb8826a20bef2769d04dd1.json new file mode 100644 index 00000000..4b670cff --- /dev/null +++ b/.sqlx/query-12dcf313d0e4c0c0da2569f3326e49e1a78fa54537cb8826a20bef2769d04dd1.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT EXISTS(\n SELECT 1\n FROM af_published_collab\n WHERE workspace_id = $1\n AND publish_name = $2\n )\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "exists", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "12dcf313d0e4c0c0da2569f3326e49e1a78fa54537cb8826a20bef2769d04dd1" +} diff --git a/.sqlx/query-2571550f7e2dd81103960741fecbf886f52f681c44344a814780b01fc06e4c8f.json b/.sqlx/query-2571550f7e2dd81103960741fecbf886f52f681c44344a814780b01fc06e4c8f.json new file mode 100644 index 00000000..199355ca --- /dev/null +++ b/.sqlx/query-2571550f7e2dd81103960741fecbf886f52f681c44344a814780b01fc06e4c8f.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT view_id\n FROM af_published_collab\n WHERE workspace_id = $1\n AND publish_name = $2\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "view_id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "2571550f7e2dd81103960741fecbf886f52f681c44344a814780b01fc06e4c8f" +} From a6af0300eeeebcba85e235f4427a3ecf6f75ce08 Mon Sep 17 00:00:00 2001 From: Zack Fu Zi Xiang Date: Tue, 29 Oct 2024 09:48:24 +0800 Subject: [PATCH 3/3] feat: add specific error code for invalid publish names --- libs/app-error/src/lib.rs | 13 +++++++++++ src/biz/workspace/publish.rs | 15 ++++++------ tests/workspace/publish.rs | 44 ++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/libs/app-error/src/lib.rs b/libs/app-error/src/lib.rs index c844e7be..d74dd029 100644 --- a/libs/app-error/src/lib.rs +++ b/libs/app-error/src/lib.rs @@ -161,6 +161,15 @@ pub enum AppError { workspace_id: Uuid, publish_name: String, }, + + #[error("There is an invalid character in the publish name: {character}")] + PublishNameInvalidCharacter { character: char }, + + #[error("The publish name is too long, given length: {given_length}, max length: {max_length}")] + PublishNameTooLong { + given_length: usize, + max_length: usize, + }, } impl AppError { @@ -232,6 +241,8 @@ impl AppError { AppError::AccessRequestAlreadyExists { .. } => ErrorCode::AccessRequestAlreadyExists, AppError::TooManyImportTask(_) => ErrorCode::TooManyImportTask, AppError::PublishNameAlreadyExists { .. } => ErrorCode::PublishNameAlreadyExists, + AppError::PublishNameInvalidCharacter { .. } => ErrorCode::PublishNameInvalidCharacter, + AppError::PublishNameTooLong { .. } => ErrorCode::PublishNameTooLong, } } } @@ -368,6 +379,8 @@ pub enum ErrorCode { CustomNamespaceTooLong = 1048, CustomNamespaceReserved = 1049, PublishNameAlreadyExists = 1050, + PublishNameInvalidCharacter = 1051, + PublishNameTooLong = 1052, } impl ErrorCode { diff --git a/src/biz/workspace/publish.rs b/src/biz/workspace/publish.rs index 69466452..9ca6149f 100644 --- a/src/biz/workspace/publish.rs +++ b/src/biz/workspace/publish.rs @@ -61,19 +61,20 @@ async fn check_workspace_owner_or_publisher( } fn check_collab_publish_name(publish_name: &str) -> Result<(), AppError> { + const MAX_PUBLISH_NAME_LENGTH: usize = 128; + // Check len - if publish_name.len() > 128 { - return Err(AppError::InvalidRequest( - "Publish name must be at most 128 characters long".to_string(), - )); + if publish_name.len() > MAX_PUBLISH_NAME_LENGTH { + return Err(AppError::PublishNameTooLong { + given_length: publish_name.len(), + max_length: MAX_PUBLISH_NAME_LENGTH, + }); } // Only contain alphanumeric characters and hyphens for c in publish_name.chars() { if !c.is_alphanumeric() && c != '-' { - return Err(AppError::InvalidRequest( - "Publish name must only contain alphanumeric characters and hyphens".to_string(), - )); + return Err(AppError::PublishNameInvalidCharacter { character: c }); } } diff --git a/tests/workspace/publish.rs b/tests/workspace/publish.rs index 952b6f7a..1f1eafa8 100644 --- a/tests/workspace/publish.rs +++ b/tests/workspace/publish.rs @@ -90,6 +90,50 @@ async fn test_publish_doc() { .await .unwrap(); + { + // Invalid publish name + let err = c + .publish_collabs::( + &workspace_id, + vec![PublishCollabItem { + meta: PublishCollabMetadata { + view_id: uuid::Uuid::new_v4(), + publish_name: "(*&^%$#!".to_string(), // invalid chars + metadata: MyCustomMetadata { + title: "my_title_1".to_string(), + }, + }, + data: "yrs_encoded_data_1".as_bytes(), + }], + ) + .await + .unwrap_err(); + assert_eq!( + err.code, + ErrorCode::PublishNameInvalidCharacter, + "{:?}", + err + ); + // Publish name too long + let err = c + .publish_collabs::( + &workspace_id, + vec![PublishCollabItem { + meta: PublishCollabMetadata { + view_id: uuid::Uuid::new_v4(), + publish_name: "a".repeat(1001), // too long + metadata: MyCustomMetadata { + title: "my_title_1".to_string(), + }, + }, + data: "yrs_encoded_data_1".as_bytes(), + }], + ) + .await + .unwrap_err(); + assert_eq!(err.code, ErrorCode::PublishNameTooLong, "{:?}", err); + } + let publish_name_1 = "publish-name-1"; let view_id_1 = uuid::Uuid::new_v4(); let publish_name_2 = "publish-name-2";