From e9e483291ecec7d3b7337fe84282a78d93b9d97c Mon Sep 17 00:00:00 2001 From: Richard Shiue <71320345+richardshiue@users.noreply.github.com> Date: Mon, 15 Apr 2024 11:43:02 +0800 Subject: [PATCH] chore: move responsibility of url valdiation to front-frontend (#5129) * chore: move responsibility of url valdiation to frontend * chore: fix test * chore: fix tauri build --- .../application/database/cell/cell_types.ts | 14 +- .../database/components/cell/URLCell.tsx | 10 +- .../type_option_entities/url_entities.rs | 3 - .../text_type_option/text_type_option.rs | 38 +---- .../type_options/url_type_option/url_tests.rs | 149 +----------------- .../url_type_option/url_type_option.rs | 49 ++---- .../url_type_option_entities.rs | 18 +-- .../group/controller_impls/url_controller.rs | 2 +- .../tests/database/cell_test/test.rs | 5 +- 9 files changed, 34 insertions(+), 254 deletions(-) diff --git a/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_types.ts b/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_types.ts index 3c999ca45b..f36f68ad8b 100644 --- a/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_types.ts +++ b/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_types.ts @@ -34,12 +34,7 @@ export interface CheckboxCell extends Cell { export interface UrlCell extends Cell { fieldType: FieldType.URL; - data: UrlCellData; -} - -export interface UrlCellData { - url: string; - content?: string; + data: string; } export interface SelectCell extends Cell { @@ -126,10 +121,9 @@ export const pbToSelectCellData = (pb: SelectOptionCellDataPB): SelectCellData = }; }; -const pbToURLCellData = (pb: URLCellDataPB): UrlCellData => ({ - url: pb.url, - content: pb.content, -}); +const pbToURLCellData = (pb: URLCellDataPB): string => ( + pb.content +); export const pbToChecklistCellData = (pb: ChecklistCellDataPB): ChecklistCellData => ({ selectedOptions: pb.selected_options.map(({ id }) => id), diff --git a/frontend/appflowy_tauri/src/appflowy_app/components/database/components/cell/URLCell.tsx b/frontend/appflowy_tauri/src/appflowy_app/components/database/components/cell/URLCell.tsx index c041cbde97..592850ada1 100644 --- a/frontend/appflowy_tauri/src/appflowy_app/components/database/components/cell/URLCell.tsx +++ b/frontend/appflowy_tauri/src/appflowy_app/components/database/components/cell/URLCell.tsx @@ -16,7 +16,7 @@ function UrlCell({ field, cell, placeholder }: Props) { const cellRef = useRef(null); const { value, editing, updateCell, setEditing, setValue } = useInputCell(cell); const handleClick = useCallback(() => { - setValue(cell.data.content || ''); + setValue(cell.data); setEditing(true); }, [cell, setEditing, setValue]); @@ -32,19 +32,17 @@ function UrlCell({ field, cell, placeholder }: Props) { ); const content = useMemo(() => { - const str = cell.data.content; - - if (str) { + if (cell.data) { return ( { e.stopPropagation(); - openUrl(str); + openUrl(cell.data); }} target={'_blank'} className={'cursor-pointer text-content-blue-400 underline'} > - {str} + {cell.data} ); } diff --git a/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/url_entities.rs b/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/url_entities.rs index 2500145fc8..1a14299d01 100644 --- a/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/url_entities.rs +++ b/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/url_entities.rs @@ -4,9 +4,6 @@ use flowy_derive::ProtoBuf; #[derive(Clone, Debug, Default, ProtoBuf)] pub struct URLCellDataPB { #[pb(index = 1)] - pub url: String, - - #[pb(index = 2)] pub content: String, } diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/text_type_option/text_type_option.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/text_type_option/text_type_option.rs index c89b3cda81..02b5b07806 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/text_type_option/text_type_option.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/text_type_option/text_type_option.rs @@ -1,6 +1,5 @@ use std::cmp::Ordering; -use bytes::Bytes; use collab::core::any_map::AnyMapExtension; use collab_database::fields::{Field, TypeOptionData, TypeOptionDataBuilder}; use collab_database::rows::{new_cell_builder, Cell}; @@ -9,9 +8,7 @@ use serde::{Deserialize, Serialize}; use flowy_error::{FlowyError, FlowyResult}; use crate::entities::{FieldType, TextFilterPB}; -use crate::services::cell::{ - stringify_cell, CellDataChangeset, CellDataDecoder, CellProtobufBlobParser, -}; +use crate::services::cell::{stringify_cell, CellDataChangeset, CellDataDecoder}; use crate::services::field::type_options::util::ProtobufStr; use crate::services::field::{ TypeOption, TypeOptionCellData, TypeOptionCellDataCompare, TypeOptionCellDataFilter, @@ -146,39 +143,6 @@ impl TypeOptionCellDataCompare for RichTextTypeOption { } } -#[derive(Clone)] -pub struct TextCellData(pub String); -impl AsRef for TextCellData { - fn as_ref(&self) -> &str { - &self.0 - } -} - -impl std::ops::Deref for TextCellData { - type Target = String; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl ToString for TextCellData { - fn to_string(&self) -> String { - self.0.clone() - } -} - -pub struct TextCellDataParser(); -impl CellProtobufBlobParser for TextCellDataParser { - type Object = TextCellData; - fn parser(bytes: &Bytes) -> FlowyResult { - match String::from_utf8(bytes.to_vec()) { - Ok(s) => Ok(TextCellData(s)), - Err(_) => Ok(TextCellData("".to_owned())), - } - } -} - #[derive(Default, Debug, Clone)] pub struct StrCellData(pub String); impl std::ops::Deref for StrCellData { diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_tests.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_tests.rs index 99a8bbf44a..7f32aa4c18 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_tests.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_tests.rs @@ -7,161 +7,26 @@ mod tests { use crate::services::field::FieldBuilder; use crate::services::field::URLTypeOption; - /// The expected_str will equal to the input string, but the expected_url will be empty if there's no - /// http url in the input string. #[test] - fn url_type_option_does_not_contain_url_test() { - let type_option = URLTypeOption::default(); - let field_type = FieldType::URL; - let field = FieldBuilder::from_field_type(field_type).build(); - assert_url(&type_option, "123", "123", "", &field); - assert_url(&type_option, "", "", "", &field); - } - - /// The expected_str will equal to the input string, but the expected_url will not be empty - /// if there's a http url in the input string. - #[test] - fn url_type_option_contains_url_test() { + fn url_test() { let type_option = URLTypeOption::default(); let field_type = FieldType::URL; let field = FieldBuilder::from_field_type(field_type).build(); assert_url( &type_option, - "AppFlowy website - https://www.appflowy.io", - "AppFlowy website - https://www.appflowy.io", - "https://www.appflowy.io/", - &field, - ); - - assert_url( - &type_option, - "AppFlowy website appflowy.io", - "AppFlowy website appflowy.io", - "https://appflowy.io", + "https://www.appflowy.io", + "https://www.appflowy.io", &field, ); + assert_url(&type_option, "123", "123", &field); + assert_url(&type_option, "", "", &field); } - /// if there's a http url and some words following it in the input string. - #[test] - fn url_type_option_contains_url_with_string_after_test() { - let type_option = URLTypeOption::default(); - let field_type = FieldType::URL; - let field = FieldBuilder::from_field_type(field_type).build(); - assert_url( - &type_option, - "AppFlowy website - https://www.appflowy.io welcome!", - "AppFlowy website - https://www.appflowy.io welcome!", - "https://www.appflowy.io/", - &field, - ); - - assert_url( - &type_option, - "AppFlowy website appflowy.io welcome!", - "AppFlowy website appflowy.io welcome!", - "https://appflowy.io", - &field, - ); - } - - /// if there's a http url and special words following it in the input string. - #[test] - fn url_type_option_contains_url_with_special_string_after_test() { - let type_option = URLTypeOption::default(); - let field_type = FieldType::URL; - let field = FieldBuilder::from_field_type(field_type).build(); - assert_url( - &type_option, - "AppFlowy website - https://www.appflowy.io!", - "AppFlowy website - https://www.appflowy.io!", - "https://www.appflowy.io/", - &field, - ); - - assert_url( - &type_option, - "AppFlowy website appflowy.io!", - "AppFlowy website appflowy.io!", - "https://appflowy.io", - &field, - ); - } - - /// if there's a level4 url in the input string. - #[test] - fn level4_url_type_test() { - let type_option = URLTypeOption::default(); - let field_type = FieldType::URL; - let field = FieldBuilder::from_field_type(field_type).build(); - assert_url( - &type_option, - "test - https://tester.testgroup.appflowy.io", - "test - https://tester.testgroup.appflowy.io", - "https://tester.testgroup.appflowy.io/", - &field, - ); - - assert_url( - &type_option, - "test tester.testgroup.appflowy.io", - "test tester.testgroup.appflowy.io", - "https://tester.testgroup.appflowy.io", - &field, - ); - } - - /// urls with different top level domains. - #[test] - fn different_top_level_domains_test() { - let type_option = URLTypeOption::default(); - let field_type = FieldType::URL; - let field = FieldBuilder::from_field_type(field_type).build(); - assert_url( - &type_option, - "appflowy - https://appflowy.com", - "appflowy - https://appflowy.com", - "https://appflowy.com/", - &field, - ); - - assert_url( - &type_option, - "appflowy - https://appflowy.top", - "appflowy - https://appflowy.top", - "https://appflowy.top/", - &field, - ); - - assert_url( - &type_option, - "appflowy - https://appflowy.net", - "appflowy - https://appflowy.net", - "https://appflowy.net/", - &field, - ); - - assert_url( - &type_option, - "appflowy - https://appflowy.edu", - "appflowy - https://appflowy.edu", - "https://appflowy.edu/", - &field, - ); - } - - fn assert_url( - type_option: &URLTypeOption, - input_str: &str, - expected_str: &str, - expected_url: &str, - _field: &Field, - ) { + fn assert_url(type_option: &URLTypeOption, input_str: &str, expected_url: &str, _field: &Field) { let decode_cell_data = type_option .apply_changeset(input_str.to_owned(), None) .unwrap() .1; - assert_eq!(expected_str.to_owned(), decode_cell_data.data); - assert_eq!(expected_url.to_owned(), decode_cell_data.url); + assert_eq!(expected_url.to_owned(), decode_cell_data.data); } } diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option.rs index 4bb1c3b533..3a95c6bae0 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option.rs @@ -1,3 +1,11 @@ +use std::cmp::Ordering; + +use collab::core::any_map::AnyMapExtension; +use collab_database::fields::{TypeOptionData, TypeOptionDataBuilder}; +use collab_database::rows::Cell; +use flowy_error::FlowyResult; +use serde::{Deserialize, Serialize}; + use crate::entities::{TextFilterPB, URLCellDataPB}; use crate::services::cell::{CellDataChangeset, CellDataDecoder}; use crate::services::field::{ @@ -6,15 +14,6 @@ use crate::services::field::{ }; use crate::services::sort::SortCondition; -use collab::core::any_map::AnyMapExtension; -use collab_database::fields::{TypeOptionData, TypeOptionDataBuilder}; -use collab_database::rows::Cell; -use fancy_regex::Regex; -use flowy_error::FlowyResult; -use lazy_static::lazy_static; -use serde::{Deserialize, Serialize}; -use std::cmp::Ordering; - #[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct URLTypeOption { pub url: String, @@ -82,14 +81,7 @@ impl CellDataChangeset for URLTypeOption { changeset: ::CellChangeset, _cell: Option, ) -> FlowyResult<(Cell, ::CellData)> { - let mut url = "".to_string(); - if let Ok(Some(m)) = URL_REGEX.find(&changeset) { - url = auto_append_scheme(m.as_str()); - } - let url_cell_data = URLCellData { - url, - data: changeset, - }; + let url_cell_data = URLCellData { data: changeset }; Ok((url_cell_data.clone().into(), url_cell_data)) } } @@ -124,26 +116,3 @@ impl TypeOptionCellDataCompare for URLTypeOption { } } } - -fn auto_append_scheme(s: &str) -> String { - // Only support https scheme by now - match url::Url::parse(s) { - Ok(url) => { - if url.scheme() == "https" { - url.into() - } else { - format!("https://{}", s) - } - }, - Err(_) => { - format!("https://{}", s) - }, - } -} - -lazy_static! { - static ref URL_REGEX: Regex = Regex::new( - "[(http(s)?):\\/\\/(www\\.)?a-zA-Z0-9@:%._\\+~#=]{2,256}\\.[a-z]{2,6}\\b([-a-zA-Z0-9@:%_\\+.~#?&//=]*)" - ) - .unwrap(); -} diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option_entities.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option_entities.rs index e378990146..2b286e0604 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option_entities.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/url_type_option/url_type_option_entities.rs @@ -11,14 +11,12 @@ use crate::services::field::{TypeOptionCellData, CELL_DATA}; #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct URLCellData { - pub url: String, pub data: String, } impl URLCellData { pub fn new(s: &str) -> Self { Self { - url: "".to_string(), data: s.to_string(), } } @@ -36,16 +34,14 @@ impl TypeOptionCellData for URLCellData { impl From<&Cell> for URLCellData { fn from(cell: &Cell) -> Self { - let url = cell.get_str_value("url").unwrap_or_default(); - let content = cell.get_str_value(CELL_DATA).unwrap_or_default(); - Self { url, data: content } + let data = cell.get_str_value(CELL_DATA).unwrap_or_default(); + Self { data } } } impl From for Cell { fn from(data: URLCellData) -> Self { new_cell_builder(FieldType::URL) - .insert_str_value("url", data.url) .insert_str_value(CELL_DATA, data.data) .build() } @@ -53,19 +49,13 @@ impl From for Cell { impl From for URLCellDataPB { fn from(data: URLCellData) -> Self { - Self { - url: data.url, - content: data.data, - } + Self { content: data.data } } } impl From for URLCellData { fn from(data: URLCellDataPB) -> Self { - Self { - url: data.url, - data: data.content, - } + Self { data: data.content } } } diff --git a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/url_controller.rs b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/url_controller.rs index 195bae405c..9d9a0468cb 100644 --- a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/url_controller.rs +++ b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/url_controller.rs @@ -54,7 +54,7 @@ impl GroupCustomize for URLGroupController { ) -> FlowyResult<(Option, Option)> { // Just return if the group with this url already exists let mut inserted_group = None; - if self.context.get_group(&_cell_data.url).is_none() { + if self.context.get_group(&_cell_data.content).is_none() { let cell_data: URLCellData = _cell_data.clone().into(); let group = Group::new(cell_data.data); let mut new_group = self.context.add_new_group(group)?; diff --git a/frontend/rust-lib/flowy-database2/tests/database/cell_test/test.rs b/frontend/rust-lib/flowy-database2/tests/database/cell_test/test.rs index 2049e58153..9f7f531771 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/cell_test/test.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/cell_test/test.rs @@ -110,7 +110,10 @@ async fn url_cell_data_test() { if let Some(cell) = row_cell.cell.as_ref() { let cell = URLCellData::from(cell); if i == 0 { - assert_eq!(cell.url.as_str(), "https://www.appflowy.io/"); + assert_eq!( + cell.data.as_str(), + "AppFlowy website - https://www.appflowy.io" + ); } } }