From 1cbb5052f17422ae1fb3939960968907d5ab9286 Mon Sep 17 00:00:00 2001 From: "Nathan.fooo" <86001920+appflowy@users.noreply.github.com> Date: Mon, 28 Oct 2024 17:21:34 +0800 Subject: [PATCH 1/2] chore: revert local host test setting (#942) --- libs/client-api-test/src/client.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/libs/client-api-test/src/client.rs b/libs/client-api-test/src/client.rs index 908d6982..e37082fc 100644 --- a/libs/client-api-test/src/client.rs +++ b/libs/client-api-test/src/client.rs @@ -5,28 +5,28 @@ use std::env; use tracing::warn; use uuid::Uuid; // -// #[cfg(not(target_arch = "wasm32"))] -// lazy_static! { -// pub static ref LOCALHOST_URL: Cow<'static, str> = -// get_env_var("LOCALHOST_URL", "http://localhost:8000"); -// pub static ref LOCALHOST_WS: Cow<'static, str> = -// get_env_var("LOCALHOST_WS", "ws://localhost:8000/ws/v1"); -// pub static ref LOCALHOST_GOTRUE: Cow<'static, str> = -// get_env_var("LOCALHOST_GOTRUE", "http://localhost:9999"); -// } - -// Use following configuration when using local server with nginx -// #[cfg(not(target_arch = "wasm32"))] lazy_static! { pub static ref LOCALHOST_URL: Cow<'static, str> = - get_env_var("LOCALHOST_URL", "http://localhost"); + get_env_var("LOCALHOST_URL", "http://localhost:8000"); pub static ref LOCALHOST_WS: Cow<'static, str> = - get_env_var("LOCALHOST_WS", "ws://localhost/ws/v1"); + get_env_var("LOCALHOST_WS", "ws://localhost:8000/ws/v1"); pub static ref LOCALHOST_GOTRUE: Cow<'static, str> = - get_env_var("LOCALHOST_GOTRUE", "http://localhost/gotrue"); + get_env_var("LOCALHOST_GOTRUE", "http://localhost:9999"); } +// Use following configuration when using local server with nginx +// +// #[cfg(not(target_arch = "wasm32"))] +// lazy_static! { +// pub static ref LOCALHOST_URL: Cow<'static, str> = +// get_env_var("LOCALHOST_URL", "http://localhost"); +// pub static ref LOCALHOST_WS: Cow<'static, str> = +// get_env_var("LOCALHOST_WS", "ws://localhost/ws/v1"); +// pub static ref LOCALHOST_GOTRUE: Cow<'static, str> = +// get_env_var("LOCALHOST_GOTRUE", "http://localhost/gotrue"); +// } + // The env vars are not available in wasm32-unknown-unknown #[cfg(target_arch = "wasm32")] lazy_static! { From 56c55fb6a95f03f727091042f77bab3c565dc3c7 Mon Sep 17 00:00:00 2001 From: Zack Date: Tue, 29 Oct 2024 09:39:41 +0800 Subject: [PATCH 2/2] feat: handle duplicated publish names for a workspace (#944) * feat: handle duplicated publish names for a workspace * chore: cargo sqlx --- ...e49e1a78fa54537cb8826a20bef2769d04dd1.json | 23 ++++++++ ...bf886f52f681c44344a814780b01fc06e4c8f.json | 23 ++++++++ libs/app-error/src/lib.rs | 8 +++ libs/database/src/workspace.rs | 46 ++++++++++++++++ src/biz/workspace/publish.rs | 55 +++++++++++++++++++ tests/workspace/publish.rs | 35 ++++++++++++ 6 files changed, 190 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" +} 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