From a4a2a4088be2a8e9fd59daad7526a3e04c815ad3 Mon Sep 17 00:00:00 2001 From: Richard Shiue <71320345+richardshiue@users.noreply.github.com> Date: Thu, 22 Feb 2024 07:12:52 +0800 Subject: [PATCH] refactor: use BoxAny for dynamically-typed cell changesets (#4655) * refactor: use BoxAny for dynamically-typed cell changesets * fix: rust-lib tests and clippy --- .../application/cell/date_cell_service.dart | 4 +- .../application/database/cell/cell_service.ts | 4 +- .../event-integration/src/database_event.rs | 2 +- .../tests/database/local_test/test.rs | 10 +- .../type_option_entities/date_entities.rs | 2 +- .../flowy-database2/src/event_handler.rs | 21 +-- .../rust-lib/flowy-database2/src/event_map.rs | 4 +- .../src/services/cell/cell_operation.rs | 122 ++++++------------ .../src/services/database/database_editor.rs | 24 ++-- .../checklist_entities.rs | 19 +-- .../date_type_option_entities.rs | 21 +-- .../src/services/field/type_options/mod.rs | 1 + .../select_type_option.rs | 22 +--- .../field/type_options/type_option.rs | 9 +- .../field/type_options/type_option_cell.rs | 16 +-- .../group/controller_impls/date_controller.rs | 2 +- .../select_option_controller/util.rs | 2 +- .../tests/database/cell_test/script.rs | 32 +++-- .../tests/database/cell_test/test.rs | 60 +++++---- .../tests/database/database_editor.rs | 30 ++--- .../tests/database/field_test/util.rs | 20 +-- .../tests/database/group_test/script.rs | 2 +- 22 files changed, 161 insertions(+), 268 deletions(-) diff --git a/frontend/appflowy_flutter/lib/plugins/database/application/cell/date_cell_service.dart b/frontend/appflowy_flutter/lib/plugins/database/application/cell/date_cell_service.dart index 3830e5fd00..6bd12a66f7 100644 --- a/frontend/appflowy_flutter/lib/plugins/database/application/cell/date_cell_service.dart +++ b/frontend/appflowy_flutter/lib/plugins/database/application/cell/date_cell_service.dart @@ -26,7 +26,7 @@ final class DateCellBackendService { String? endTime, String? reminderId, }) { - final payload = DateChangesetPB.create() + final payload = DateCellChangesetPB.create() ..cellId = cellId ..includeTime = includeTime ..isRange = isRange; @@ -53,7 +53,7 @@ final class DateCellBackendService { } Future> clear() { - final payload = DateChangesetPB.create() + final payload = DateCellChangesetPB.create() ..cellId = cellId ..clearFlag = true; diff --git a/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_service.ts b/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_service.ts index c1ca8d7583..edb51fc97e 100644 --- a/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_service.ts +++ b/frontend/appflowy_tauri/src/appflowy_app/application/database/cell/cell_service.ts @@ -3,7 +3,7 @@ import { CellChangesetPB, SelectOptionCellChangesetPB, ChecklistCellDataChangesetPB, - DateChangesetPB, + DateCellChangesetPB, FieldType, } from '../../../../services/backend'; import { @@ -115,7 +115,7 @@ export async function updateDateCell( isRange?: boolean; } ): Promise { - const payload = DateChangesetPB.fromObject({ + const payload = DateCellChangesetPB.fromObject({ cell_id: { view_id: viewId, row_id: rowId, diff --git a/frontend/rust-lib/event-integration/src/database_event.rs b/frontend/rust-lib/event-integration/src/database_event.rs index ff680af0dd..ec25fafef2 100644 --- a/frontend/rust-lib/event-integration/src/database_event.rs +++ b/frontend/rust-lib/event-integration/src/database_event.rs @@ -298,7 +298,7 @@ impl EventIntegrationTest { .error() } - pub async fn update_date_cell(&self, changeset: DateChangesetPB) -> Option { + pub async fn update_date_cell(&self, changeset: DateCellChangesetPB) -> Option { EventBuilder::new(self.clone()) .event(DatabaseEvent::UpdateDateCell) .payload(changeset) diff --git a/frontend/rust-lib/event-integration/tests/database/local_test/test.rs b/frontend/rust-lib/event-integration/tests/database/local_test/test.rs index 0e593876eb..c05b00a8b8 100644 --- a/frontend/rust-lib/event-integration/tests/database/local_test/test.rs +++ b/frontend/rust-lib/event-integration/tests/database/local_test/test.rs @@ -6,8 +6,8 @@ use event_integration::event_builder::EventBuilder; use event_integration::EventIntegrationTest; use flowy_database2::entities::{ CellChangesetPB, CellIdPB, CheckboxCellDataPB, ChecklistCellDataChangesetPB, DatabaseLayoutPB, - DatabaseSettingChangesetPB, DatabaseViewIdPB, DateChangesetPB, FieldType, OrderObjectPositionPB, - SelectOptionCellDataPB, UpdateRowMetaChangesetPB, + DatabaseSettingChangesetPB, DatabaseViewIdPB, DateCellChangesetPB, FieldType, + OrderObjectPositionPB, SelectOptionCellDataPB, UpdateRowMetaChangesetPB, }; use lib_infra::util::timestamp; @@ -529,7 +529,7 @@ async fn update_date_cell_event_test() { // Insert data into the date cell of the first row. let timestamp = 1686300557; let error = test - .update_date_cell(DateChangesetPB { + .update_date_cell(DateCellChangesetPB { cell_id: cell_path, date: Some(timestamp), ..Default::default() @@ -565,7 +565,7 @@ async fn update_date_cell_event_with_empty_time_str_test() { // Insert empty timestamp string let error = test - .update_date_cell(DateChangesetPB { + .update_date_cell(DateCellChangesetPB { cell_id: cell_path, date: None, ..Default::default() @@ -763,7 +763,7 @@ async fn create_calendar_event_test() { // Insert data into the date cell of the first row. let error = test - .update_date_cell(DateChangesetPB { + .update_date_cell(DateCellChangesetPB { cell_id: CellIdPB { view_id: calendar_view.id.clone(), field_id: date_field.id.clone(), diff --git a/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/date_entities.rs b/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/date_entities.rs index 2f2c1453da..5fdb0195ef 100644 --- a/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/date_entities.rs +++ b/frontend/rust-lib/flowy-database2/src/entities/type_option_entities/date_entities.rs @@ -38,7 +38,7 @@ pub struct DateCellDataPB { } #[derive(Clone, Debug, Default, ProtoBuf)] -pub struct DateChangesetPB { +pub struct DateCellChangesetPB { #[pb(index = 1)] pub cell_id: CellIdPB, diff --git a/frontend/rust-lib/flowy-database2/src/event_handler.rs b/frontend/rust-lib/flowy-database2/src/event_handler.rs index 3993f8f809..455cecabee 100644 --- a/frontend/rust-lib/flowy-database2/src/event_handler.rs +++ b/frontend/rust-lib/flowy-database2/src/event_handler.rs @@ -2,6 +2,7 @@ use std::sync::{Arc, Weak}; use collab_database::database::gen_row_id; use collab_database::rows::RowId; +use lib_infra::box_any::BoxAny; use tokio::sync::oneshot; use flowy_error::{FlowyError, FlowyResult}; @@ -11,9 +12,8 @@ use lib_infra::util::timestamp; use crate::entities::*; use crate::manager::DatabaseManager; use crate::services::cell::CellBuilder; -use crate::services::field::checklist_type_option::ChecklistCellChangeset; use crate::services::field::{ - type_option_data_from_pb, DateCellChangeset, SelectOptionCellChangeset, + type_option_data_from_pb, ChecklistCellChangeset, DateCellChangeset, SelectOptionCellChangeset, }; use crate::services::field_settings::FieldSettingsChangesetParams; use crate::services::group::GroupChangeset; @@ -454,7 +454,7 @@ pub(crate) async fn update_cell_handler( ¶ms.view_id, RowId::from(params.row_id), ¶ms.field_id, - params.cell_changeset.clone(), + BoxAny::new(params.cell_changeset), ) .await?; Ok(()) @@ -551,7 +551,7 @@ pub(crate) async fn update_select_option_cell_handler( ¶ms.cell_identifier.view_id, params.cell_identifier.row_id, ¶ms.cell_identifier.field_id, - changeset, + BoxAny::new(changeset), ) .await?; Ok(()) @@ -572,14 +572,19 @@ pub(crate) async fn update_checklist_cell_handler( update_options: params.update_options, }; database_editor - .update_cell_with_changeset(¶ms.view_id, params.row_id, ¶ms.field_id, changeset) + .update_cell_with_changeset( + ¶ms.view_id, + params.row_id, + ¶ms.field_id, + BoxAny::new(changeset), + ) .await?; Ok(()) } #[tracing::instrument(level = "trace", skip_all, err)] pub(crate) async fn update_date_cell_handler( - data: AFPluginData, + data: AFPluginData, manager: AFPluginState>, ) -> Result<(), FlowyError> { let manager = upgrade_manager(manager)?; @@ -602,7 +607,7 @@ pub(crate) async fn update_date_cell_handler( &cell_id.view_id, cell_id.row_id, &cell_id.field_id, - cell_changeset, + BoxAny::new(cell_changeset), ) .await?; Ok(()) @@ -831,7 +836,7 @@ pub(crate) async fn move_calendar_event_handler( &cell_id.view_id, cell_id.row_id, &cell_id.field_id, - cell_changeset, + BoxAny::new(cell_changeset), ) .await?; Ok(()) diff --git a/frontend/rust-lib/flowy-database2/src/event_map.rs b/frontend/rust-lib/flowy-database2/src/event_map.rs index c2e7f10802..94ab6d49ed 100644 --- a/frontend/rust-lib/flowy-database2/src/event_map.rs +++ b/frontend/rust-lib/flowy-database2/src/event_map.rs @@ -256,10 +256,10 @@ pub enum DatabaseEvent { #[event(input = "ChecklistCellDataChangesetPB")] UpdateChecklistCell = 73, - /// [UpdateDateCell] event is used to update a date cell's data. [DateChangesetPB] + /// [UpdateDateCell] event is used to update a date cell's data. [DateCellChangesetPB] /// contains the date and the time string. It can be cast to [CellChangesetPB] that /// will be used by the `update_cell` function. - #[event(input = "DateChangesetPB")] + #[event(input = "DateCellChangesetPB")] UpdateDateCell = 80, /// [SetGroupByField] event is used to create a new grouping in a database diff --git a/frontend/rust-lib/flowy-database2/src/services/cell/cell_operation.rs b/frontend/rust-lib/flowy-database2/src/services/cell/cell_operation.rs index d68cf4fb10..354c9546c4 100644 --- a/frontend/rust-lib/flowy-database2/src/services/cell/cell_operation.rs +++ b/frontend/rust-lib/flowy-database2/src/services/cell/cell_operation.rs @@ -1,14 +1,13 @@ use std::collections::HashMap; -use std::fmt::Debug; use collab_database::fields::Field; use collab_database::rows::{get_field_type_from_cell, Cell, Cells}; -use flowy_error::{ErrorCode, FlowyError, FlowyResult}; +use flowy_error::{FlowyError, FlowyResult}; +use lib_infra::box_any::BoxAny; use crate::entities::{CheckboxCellDataPB, FieldType}; use crate::services::cell::{CellCache, CellProtobufBlob}; -use crate::services::field::checklist_type_option::ChecklistCellChangeset; use crate::services::field::*; use crate::services::group::make_no_status_group; @@ -48,8 +47,9 @@ pub trait CellDataDecoder: TypeOption { pub trait CellDataChangeset: TypeOption { /// The changeset is able to parse into the concrete data struct if `TypeOption::CellChangeset` - /// implements the `FromCellChangesetString` trait. - /// For example,the SelectOptionCellChangeset,DateCellChangeset. etc. + /// implements the `FromCellChangesetString` trait e.g. + /// SelectOptionCellChangeset, DateCellChangeset + /// /// # Arguments /// /// * `changeset`: the cell changeset that represents the changes of the cell. @@ -67,13 +67,12 @@ pub trait CellDataChangeset: TypeOption { /// FieldType::SingleSelect => SelectOptionChangeset /// /// cell_rev: It will be None if the cell does not contain any data. -pub fn apply_cell_changeset( - changeset: C, +pub fn apply_cell_changeset( + changeset: BoxAny, cell: Option, field: &Field, cell_data_cache: Option, ) -> Result { - let changeset = changeset.to_cell_changeset_str(); let field_type = FieldType::from(field.field_type); match TypeOptionCellExt::new_with_cell_data_cache(field, cell_data_cache) .get_type_option_cell_data_handler(&field_type) @@ -155,6 +154,7 @@ pub fn try_decode_cell_to_cell_data( .ok()? .unbox_or_none::() } + /// Returns a string that represents the current field_type's cell data. /// For example, The string of the Multi-Select cell will be a list of the option's name /// separated by a comma. @@ -182,11 +182,11 @@ pub fn stringify_cell_data( } pub fn insert_text_cell(s: String, field: &Field) -> Cell { - apply_cell_changeset(s, None, field, None).unwrap() + apply_cell_changeset(BoxAny::new(s), None, field, None).unwrap() } pub fn insert_number_cell(num: i64, field: &Field) -> Cell { - apply_cell_changeset(num.to_string(), None, field, None).unwrap() + apply_cell_changeset(BoxAny::new(num.to_string()), None, field, None).unwrap() } pub fn insert_url_cell(url: String, field: &Field) -> Cell { @@ -200,7 +200,7 @@ pub fn insert_url_cell(url: String, field: &Field) -> Cell { _ => url, }; - apply_cell_changeset(url, None, field, None).unwrap() + apply_cell_changeset(BoxAny::new(url), None, field, None).unwrap() } pub fn insert_checkbox_cell(is_checked: bool, field: &Field) -> Cell { @@ -209,38 +209,40 @@ pub fn insert_checkbox_cell(is_checked: bool, field: &Field) -> Cell { } else { UNCHECK.to_string() }; - apply_cell_changeset(s, None, field, None).unwrap() + apply_cell_changeset(BoxAny::new(s), None, field, None).unwrap() } -pub fn insert_date_cell(timestamp: i64, include_time: Option, field: &Field) -> Cell { - let cell_data = serde_json::to_string(&DateCellChangeset { +pub fn insert_date_cell( + timestamp: i64, + time: Option, + include_time: Option, + field: &Field, +) -> Cell { + let cell_data = DateCellChangeset { date: Some(timestamp), + time, include_time, ..Default::default() - }) - .unwrap(); - apply_cell_changeset(cell_data, None, field, None).unwrap() + }; + apply_cell_changeset(BoxAny::new(cell_data), None, field, None).unwrap() } pub fn insert_select_option_cell(option_ids: Vec, field: &Field) -> Cell { - let changeset = - SelectOptionCellChangeset::from_insert_options(option_ids).to_cell_changeset_str(); - apply_cell_changeset(changeset, None, field, None).unwrap() + let changeset = SelectOptionCellChangeset::from_insert_options(option_ids); + apply_cell_changeset(BoxAny::new(changeset), None, field, None).unwrap() } pub fn insert_checklist_cell(insert_options: Vec, field: &Field) -> Cell { let changeset = ChecklistCellChangeset { insert_options, ..Default::default() - } - .to_cell_changeset_str(); - apply_cell_changeset(changeset, None, field, None).unwrap() + }; + apply_cell_changeset(BoxAny::new(changeset), None, field, None).unwrap() } pub fn delete_select_option_cell(option_ids: Vec, field: &Field) -> Cell { - let changeset = - SelectOptionCellChangeset::from_delete_options(option_ids).to_cell_changeset_str(); - apply_cell_changeset(changeset, None, field, None).unwrap() + let changeset = SelectOptionCellChangeset::from_delete_options(option_ids); + apply_cell_changeset(BoxAny::new(changeset), None, field, None).unwrap() } /// Deserialize the String into cell specific data type. @@ -250,59 +252,6 @@ pub trait FromCellString { Self: Sized; } -/// If the changeset applying to the cell is not String type, it should impl this trait. -/// Deserialize the string into cell specific changeset. -pub trait FromCellChangeset { - fn from_changeset(changeset: String) -> FlowyResult - where - Self: Sized; -} - -impl FromCellChangeset for String { - fn from_changeset(changeset: String) -> FlowyResult - where - Self: Sized, - { - Ok(changeset) - } -} - -pub trait ToCellChangeset: Debug { - fn to_cell_changeset_str(&self) -> String; -} - -impl ToCellChangeset for String { - fn to_cell_changeset_str(&self) -> String { - self.clone() - } -} - -pub struct AnyCellChangeset(pub Option); - -impl AnyCellChangeset { - pub fn try_into_inner(self) -> FlowyResult { - match self.0 { - None => Err(ErrorCode::InvalidParams.into()), - Some(data) => Ok(data), - } - } -} - -impl std::convert::From for AnyCellChangeset -where - T: FromCellChangeset, -{ - fn from(changeset: C) -> Self { - match T::from_changeset(changeset.to_string()) { - Ok(data) => AnyCellChangeset(Some(data)), - Err(e) => { - tracing::error!("Deserialize CellDataChangeset failed: {}", e); - AnyCellChangeset(None) - }, - } - } -} - pub struct CellBuilder<'a> { cells: Cells, field_maps: HashMap, @@ -331,7 +280,10 @@ impl<'a> CellBuilder<'a> { }, FieldType::DateTime => { if let Ok(timestamp) = cell_str.parse::() { - cells.insert(field_id, insert_date_cell(timestamp, Some(false), field)); + cells.insert( + field_id, + insert_date_cell(timestamp, None, Some(false), field), + ); } }, FieldType::LastEditedTime | FieldType::CreatedTime => { @@ -410,13 +362,19 @@ impl<'a> CellBuilder<'a> { } } - pub fn insert_date_cell(&mut self, field_id: &str, timestamp: i64) { + pub fn insert_date_cell( + &mut self, + field_id: &str, + timestamp: i64, + time: Option, + include_time: Option, + ) { match self.field_maps.get(&field_id.to_owned()) { None => tracing::warn!("Can't find the date field with id: {}", field_id), Some(field) => { self.cells.insert( field_id.to_owned(), - insert_date_cell(timestamp, Some(false), field), + insert_date_cell(timestamp, time, include_time, field), ); }, } diff --git a/frontend/rust-lib/flowy-database2/src/services/database/database_editor.rs b/frontend/rust-lib/flowy-database2/src/services/database/database_editor.rs index 60807cc639..24f94fc9d9 100644 --- a/frontend/rust-lib/flowy-database2/src/services/database/database_editor.rs +++ b/frontend/rust-lib/flowy-database2/src/services/database/database_editor.rs @@ -6,6 +6,7 @@ use collab_database::fields::{Field, TypeOptionData}; use collab_database::rows::{Cell, Cells, CreateRowParams, Row, RowCell, RowDetail, RowId}; use collab_database::views::{DatabaseLayout, DatabaseView, LayoutSetting, OrderObjectPosition}; use futures::StreamExt; +use lib_infra::box_any::BoxAny; use tokio::sync::{broadcast, RwLock}; use tracing::{event, warn}; @@ -17,17 +18,16 @@ use lib_infra::priority_task::TaskDispatcher; use crate::entities::*; use crate::notification::{send_notification, DatabaseNotification}; use crate::services::calculations::Calculation; -use crate::services::cell::{apply_cell_changeset, get_cell_protobuf, CellCache, ToCellChangeset}; +use crate::services::cell::{apply_cell_changeset, get_cell_protobuf, CellCache}; use crate::services::database::util::database_view_setting_pb_from_view; use crate::services::database::UpdatedRow; use crate::services::database_view::{ DatabaseViewChanged, DatabaseViewEditor, DatabaseViewOperation, DatabaseViews, EditorByViewId, }; -use crate::services::field::checklist_type_option::ChecklistCellChangeset; use crate::services::field::{ default_type_option_data_from_type, select_type_option_from_field, transform_type_option, - type_option_data_from_pb, SelectOptionCellChangeset, SelectOptionIds, TimestampCellData, - TypeOptionCellDataHandler, TypeOptionCellExt, + type_option_data_from_pb, ChecklistCellChangeset, SelectOptionCellChangeset, SelectOptionIds, + TimestampCellData, TypeOptionCellDataHandler, TypeOptionCellExt, }; use crate::services::field_settings::{ default_field_settings_by_layout_map, FieldSettings, FieldSettingsChangesetParams, @@ -105,6 +105,7 @@ impl DatabaseEditor { ) .await?, ); + Ok(Self { database, cell_cache, @@ -730,16 +731,13 @@ impl DatabaseEditor { } } - pub async fn update_cell_with_changeset( + pub async fn update_cell_with_changeset( &self, view_id: &str, row_id: RowId, field_id: &str, - cell_changeset: T, - ) -> FlowyResult<()> - where - T: ToCellChangeset, - { + cell_changeset: BoxAny, + ) -> FlowyResult<()> { let (field, cell) = { let database = self.database.lock(); let field = match database.fields.get_field(field_id) { @@ -872,7 +870,7 @@ impl DatabaseEditor { // Insert the options into the cell self - .update_cell_with_changeset(view_id, row_id, field_id, cell_changeset) + .update_cell_with_changeset(view_id, row_id, field_id, BoxAny::new(cell_changeset)) .await?; Ok(()) } @@ -911,7 +909,7 @@ impl DatabaseEditor { .await?; self - .update_cell_with_changeset(view_id, row_id, field_id, cell_changeset) + .update_cell_with_changeset(view_id, row_id, field_id, BoxAny::new(cell_changeset)) .await?; Ok(()) } @@ -953,7 +951,7 @@ impl DatabaseEditor { debug_assert!(FieldType::from(field.field_type).is_checklist()); self - .update_cell_with_changeset(view_id, row_id, field_id, changeset) + .update_cell_with_changeset(view_id, row_id, field_id, BoxAny::new(changeset)) .await?; Ok(()) } diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs index d0ff75ac33..37633b0392 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/checklist_type_option/checklist_entities.rs @@ -1,9 +1,7 @@ use crate::entities::FieldType; -use crate::services::cell::{FromCellChangeset, ToCellChangeset}; use crate::services::field::{SelectOption, TypeOptionCellData, CELL_DATA}; use collab::core::any_map::AnyMapExtension; use collab_database::rows::{new_cell_builder, Cell}; -use flowy_error::{internal_error, FlowyResult}; use serde::{Deserialize, Serialize}; use std::fmt::Debug; @@ -76,7 +74,7 @@ impl From for Cell { } } -#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, Default)] pub struct ChecklistCellChangeset { /// List of option names that will be inserted pub insert_options: Vec, @@ -85,21 +83,6 @@ pub struct ChecklistCellChangeset { pub update_options: Vec, } -impl FromCellChangeset for ChecklistCellChangeset { - fn from_changeset(changeset: String) -> FlowyResult - where - Self: Sized, - { - serde_json::from_str::(&changeset).map_err(internal_error) - } -} - -impl ToCellChangeset for ChecklistCellChangeset { - fn to_cell_changeset_str(&self) -> String { - serde_json::to_string(self).unwrap_or_default() - } -} - #[cfg(test)] mod tests { #[test] diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option_entities.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option_entities.rs index 0c08073060..0bb1190768 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option_entities.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/date_type_option/date_type_option_entities.rs @@ -11,12 +11,10 @@ use strum_macros::EnumIter; use flowy_error::{internal_error, FlowyResult}; use crate::entities::{DateCellDataPB, FieldType}; -use crate::services::cell::{ - CellProtobufBlobParser, DecodedCellData, FromCellChangeset, FromCellString, ToCellChangeset, -}; +use crate::services::cell::{CellProtobufBlobParser, DecodedCellData, FromCellString}; use crate::services::field::{TypeOptionCellData, CELL_DATA}; -#[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[derive(Clone, Debug, Default)] pub struct DateCellChangeset { pub date: Option, pub time: Option, @@ -28,21 +26,6 @@ pub struct DateCellChangeset { pub reminder_id: Option, } -impl FromCellChangeset for DateCellChangeset { - fn from_changeset(changeset: String) -> FlowyResult - where - Self: Sized, - { - serde_json::from_str::(&changeset).map_err(internal_error) - } -} - -impl ToCellChangeset for DateCellChangeset { - fn to_cell_changeset_str(&self) -> String { - serde_json::to_string(self).unwrap_or_default() - } -} - #[derive(Default, Clone, Debug, Serialize)] pub struct DateCellData { pub timestamp: Option, diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/mod.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/mod.rs index 46ef5470b4..075c5c41f2 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/mod.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/mod.rs @@ -11,6 +11,7 @@ mod url_type_option; mod util; pub use checkbox_type_option::*; +pub use checklist_type_option::*; pub use date_type_option::*; pub use number_type_option::*; pub use selection_type_option::*; diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs index 2ad5127345..367284161b 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/selection_type_option/select_type_option.rs @@ -1,14 +1,11 @@ use bytes::Bytes; use collab_database::fields::{Field, TypeOptionData}; use collab_database::rows::Cell; -use serde::{Deserialize, Serialize}; use flowy_error::{internal_error, ErrorCode, FlowyResult}; use crate::entities::{CheckboxCellDataPB, FieldType, SelectOptionCellDataPB}; -use crate::services::cell::{ - CellDataDecoder, CellProtobufBlobParser, DecodedCellData, FromCellChangeset, ToCellChangeset, -}; +use crate::services::cell::{CellDataDecoder, CellProtobufBlobParser, DecodedCellData}; use crate::services::field::selection_type_option::type_option_transform::SelectOptionTypeOptionTransformHelper; use crate::services::field::{ make_selected_options, MultiSelectTypeOption, SelectOption, SelectOptionCellData, @@ -231,27 +228,12 @@ impl CellProtobufBlobParser for SelectOptionCellDataParser { } } -#[derive(Clone, Serialize, Deserialize, Default, Debug)] +#[derive(Clone, Default, Debug)] pub struct SelectOptionCellChangeset { pub insert_option_ids: Vec, pub delete_option_ids: Vec, } -impl FromCellChangeset for SelectOptionCellChangeset { - fn from_changeset(changeset: String) -> FlowyResult - where - Self: Sized, - { - serde_json::from_str::(&changeset).map_err(internal_error) - } -} - -impl ToCellChangeset for SelectOptionCellChangeset { - fn to_cell_changeset_str(&self) -> String { - serde_json::to_string(self).unwrap_or_default() - } -} - impl SelectOptionCellChangeset { pub fn from_insert_option_id(option_id: &str) -> Self { SelectOptionCellChangeset { diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs index 9cda2e2f59..f0389ff8ad 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option.rs @@ -13,7 +13,7 @@ use crate::entities::{ MultiSelectTypeOptionPB, NumberTypeOptionPB, RichTextTypeOptionPB, SingleSelectTypeOptionPB, TimestampTypeOptionPB, URLTypeOptionPB, }; -use crate::services::cell::{CellDataDecoder, FromCellChangeset, ToCellChangeset}; +use crate::services::cell::CellDataDecoder; use crate::services::field::checklist_type_option::ChecklistTypeOption; use crate::services::field::{ CheckboxTypeOption, DateTypeOption, MultiSelectTypeOption, NumberTypeOption, RichTextTypeOption, @@ -43,11 +43,10 @@ pub trait TypeOption { + Debug + 'static; - /// Represents as the corresponding field type cell changeset. - /// The changeset must implements the `FromCellChangesetString` and the `ToCellChangesetString` trait. - /// These two traits are auto implemented for `String`. + /// Represents as the corresponding field type cell changeset. Must be able + /// to be placed into a `BoxAny`. /// - type CellChangeset: FromCellChangeset + ToCellChangeset; + type CellChangeset: Send + Sync + 'static; /// For the moment, the protobuf type only be used in the FFI of `Dart`. If the decoded cell /// struct is just a `String`, then use the `StrCellData` as its `CellProtobufType`. diff --git a/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option_cell.rs b/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option_cell.rs index 95ac3f005a..f98bcd0cc0 100644 --- a/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option_cell.rs +++ b/frontend/rust-lib/flowy-database2/src/services/field/type_options/type_option_cell.rs @@ -11,7 +11,6 @@ use lib_infra::box_any::BoxAny; use crate::entities::FieldType; use crate::services::cell::{ CellCache, CellDataChangeset, CellDataDecoder, CellFilterCache, CellProtobufBlob, - FromCellChangeset, }; use crate::services::field::checklist_type_option::ChecklistTypeOption; use crate::services::field::{ @@ -26,9 +25,11 @@ pub const CELL_DATA: &str = "data"; /// Each [FieldType] has its own [TypeOptionCellDataHandler]. /// A helper trait that used to erase the `Self` of `TypeOption` trait to make it become a Object-safe trait /// Only object-safe traits can be made into trait objects. -/// > Object-safe traits are traits with methods that follow these two rules: -/// 1.the return type is not Self. -/// 2.there are no generic types parameters. +/// +/// Object-safe traits are traits with methods that follow these two rules: +/// +/// 1. the return type is not Self. +/// 2. there are no generic types parameters. /// pub trait TypeOptionCellDataHandler: Send + Sync + 'static { fn handle_cell_str( @@ -38,10 +39,9 @@ pub trait TypeOptionCellDataHandler: Send + Sync + 'static { field_rev: &Field, ) -> FlowyResult; - // TODO(nathan): replace cell_changeset with BoxAny to get rid of the serde process. fn handle_cell_changeset( &self, - cell_changeset: String, + cell_changeset: BoxAny, old_cell: Option, field: &Field, ) -> FlowyResult; @@ -238,11 +238,11 @@ where fn handle_cell_changeset( &self, - cell_changeset: String, + cell_changeset: BoxAny, old_cell: Option, field: &Field, ) -> FlowyResult { - let changeset = ::CellChangeset::from_changeset(cell_changeset)?; + let changeset = cell_changeset.unbox_or_error::<::CellChangeset>()?; let (cell, cell_data) = self.apply_changeset(changeset, old_cell)?; self.set_decoded_cell_data(&cell, cell_data, field); Ok(cell) diff --git a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/date_controller.rs b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/date_controller.rs index c7368e71e1..1d947d66e3 100644 --- a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/date_controller.rs +++ b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/date_controller.rs @@ -243,7 +243,7 @@ impl GroupController for DateGroupController { None => tracing::warn!("Can not find the group: {}", group_id), Some((_, _)) => { let date = DateTime::parse_from_str(group_id, GROUP_ID_DATE_FORMAT).unwrap(); - let cell = insert_date_cell(date.timestamp(), None, field); + let cell = insert_date_cell(date.timestamp(), None, Some(false), field); cells.insert(field.id.clone(), cell); }, } diff --git a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/util.rs b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/util.rs index 4ed745cad5..b8a144b594 100644 --- a/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/util.rs +++ b/frontend/rust-lib/flowy-database2/src/services/group/controller_impls/select_option_controller/util.rs @@ -176,7 +176,7 @@ pub fn make_inserted_cell(group_id: &str, field: &Field) -> Option { let date = NaiveDateTime::parse_from_str(&format!("{} 00:00:00", group_id), "%Y/%m/%d %H:%M:%S") .unwrap(); - let cell = insert_date_cell(date.timestamp(), None, field); + let cell = insert_date_cell(date.timestamp(), None, Some(false), field); Some(cell) }, _ => { diff --git a/frontend/rust-lib/flowy-database2/tests/database/cell_test/script.rs b/frontend/rust-lib/flowy-database2/tests/database/cell_test/script.rs index 9d2ad9937b..85f826cd4d 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/cell_test/script.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/cell_test/script.rs @@ -1,12 +1,15 @@ use collab_database::rows::RowId; -use flowy_database2::entities::CellChangesetPB; +use lib_infra::box_any::BoxAny; use crate::database::database_editor::DatabaseEditorTest; pub enum CellScript { UpdateCell { - changeset: CellChangesetPB, + view_id: String, + field_id: String, + row_id: RowId, + changeset: BoxAny, is_err: bool, }, } @@ -35,25 +38,26 @@ impl DatabaseCellTest { match script { CellScript::UpdateCell { + view_id, + field_id, + row_id, changeset, is_err: _, } => { self .editor - .update_cell_with_changeset( - &self.view_id, - RowId::from(changeset.row_id), - &changeset.field_id, - changeset.cell_changeset, - ) + .update_cell_with_changeset(&view_id, row_id, &field_id, changeset) .await .unwrap(); - }, // CellScript::AssertGridRevisionPad => { - // sleep(Duration::from_millis(2 * REVISION_WRITE_INTERVAL_IN_MILLIS)).await; - // let mut grid_rev_manager = grid_manager.make_grid_rev_manager(&self.grid_id, pool.clone()).unwrap(); - // let grid_pad = grid_rev_manager.load::(None).await.unwrap(); - // println!("{}", grid_pad.delta_str()); - // } + }, + // CellScript::AssertGridRevisionPad => { + // sleep(Duration::from_millis(2 * REVISION_WRITE_INTERVAL_IN_MILLIS)).await; + // let mut grid_rev_manager = grid_manager + // .make_grid_rev_manager(&self.grid_id, pool.clone()) + // .unwrap(); + // let grid_pad = grid_rev_manager.load::(None).await.unwrap(); + // println!("{}", grid_pad.delta_str()); + // }, } } } 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 f3bb9cf09c..ff6af7a3c0 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 @@ -1,16 +1,14 @@ use std::time::Duration; -use flowy_database2::entities::{CellChangesetPB, FieldType}; -use flowy_database2::services::cell::ToCellChangeset; -use flowy_database2::services::field::checklist_type_option::ChecklistCellChangeset; +use flowy_database2::entities::FieldType; use flowy_database2::services::field::{ - DateCellData, MultiSelectTypeOption, SelectOptionCellChangeset, SingleSelectTypeOption, - StrCellData, URLCellData, + ChecklistCellChangeset, DateCellChangeset, DateCellData, MultiSelectTypeOption, + SelectOptionCellChangeset, SingleSelectTypeOption, StrCellData, URLCellData, }; +use lib_infra::box_any::BoxAny; use crate::database::cell_test::script::CellScript::UpdateCell; use crate::database::cell_test::script::DatabaseCellTest; -use crate::database::field_test::util::make_date_cell_string; #[tokio::test] async fn grid_cell_update() { @@ -26,40 +24,42 @@ async fn grid_cell_update() { continue; } let cell_changeset = match field_type { - FieldType::RichText => "".to_string(), - FieldType::Number => "123".to_string(), - FieldType::DateTime => make_date_cell_string(123), + FieldType::RichText => BoxAny::new("".to_string()), + FieldType::Number => BoxAny::new("123".to_string()), + FieldType::DateTime => BoxAny::new(DateCellChangeset { + date: Some(123), + ..Default::default() + }), FieldType::SingleSelect => { let type_option = field .get_type_option::(field.field_type) .unwrap(); - SelectOptionCellChangeset::from_insert_option_id(&type_option.options.first().unwrap().id) - .to_cell_changeset_str() + BoxAny::new(SelectOptionCellChangeset::from_insert_option_id( + &type_option.options.first().unwrap().id, + )) }, FieldType::MultiSelect => { let type_option = field .get_type_option::(field.field_type) .unwrap(); - SelectOptionCellChangeset::from_insert_option_id(&type_option.options.first().unwrap().id) - .to_cell_changeset_str() + BoxAny::new(SelectOptionCellChangeset::from_insert_option_id( + &type_option.options.first().unwrap().id, + )) }, - FieldType::Checklist => ChecklistCellChangeset { + FieldType::Checklist => BoxAny::new(ChecklistCellChangeset { insert_options: vec!["new option".to_string()], ..Default::default() - } - .to_cell_changeset_str(), - FieldType::Checkbox => "1".to_string(), - FieldType::URL => "1".to_string(), - _ => "".to_string(), + }), + FieldType::Checkbox => BoxAny::new("1".to_string()), + FieldType::URL => BoxAny::new("1".to_string()), + _ => BoxAny::new("".to_string()), }; scripts.push(UpdateCell { - changeset: CellChangesetPB { - view_id: test.view_id.clone(), - row_id: row_detail.row.id.clone().into(), - field_id: field.id.clone(), - cell_changeset, - }, + view_id: test.view_id.clone(), + field_id: field.id.clone(), + row_id: row_detail.row.id.clone(), + changeset: cell_changeset, is_err: false, }); } @@ -125,12 +125,10 @@ async fn update_updated_at_field_on_other_cell_update() { let before_update_timestamp = chrono::offset::Utc::now().timestamp(); test .run_script(UpdateCell { - changeset: CellChangesetPB { - view_id: test.view_id.clone(), - row_id: test.row_details[0].row.id.to_string(), - field_id: text_field.id.clone(), - cell_changeset: "change".to_string(), - }, + view_id: test.view_id.clone(), + row_id: test.row_details[0].row.id.clone(), + field_id: text_field.id.clone(), + changeset: BoxAny::new("change".to_string()), is_err: false, }) .await; diff --git a/frontend/rust-lib/flowy-database2/tests/database/database_editor.rs b/frontend/rust-lib/flowy-database2/tests/database/database_editor.rs index daf576ef13..a83041bbaf 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/database_editor.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/database_editor.rs @@ -5,19 +5,20 @@ use collab_database::database::{gen_database_view_id, timestamp}; use collab_database::fields::Field; use collab_database::rows::{CreateRowParams, RowDetail, RowId}; use collab_database::views::OrderObjectPosition; +use lib_infra::box_any::BoxAny; use strum::EnumCount; use event_integration::folder_event::ViewTest; use event_integration::EventIntegrationTest; use flowy_database2::entities::{FieldType, FilterPB, RowMetaPB}; -use flowy_database2::services::cell::{CellBuilder, ToCellChangeset}; +use flowy_database2::services::cell::CellBuilder; use flowy_database2::services::database::DatabaseEditor; use flowy_database2::services::field::checklist_type_option::{ ChecklistCellChangeset, ChecklistTypeOption, }; use flowy_database2::services::field::{ - CheckboxTypeOption, DateCellChangeset, MultiSelectTypeOption, SelectOption, - SelectOptionCellChangeset, SingleSelectTypeOption, + CheckboxTypeOption, MultiSelectTypeOption, SelectOption, SelectOptionCellChangeset, + SingleSelectTypeOption, }; use flowy_database2::services::share::csv::{CSVFormat, ImportResult}; use flowy_error::FlowyResult; @@ -180,11 +181,11 @@ impl DatabaseEditorTest { .unwrap() } - pub async fn update_cell( + pub async fn update_cell( &mut self, field_id: &str, row_id: RowId, - cell_changeset: T, + cell_changeset: BoxAny, ) -> FlowyResult<()> { let field = self .editor @@ -212,7 +213,7 @@ impl DatabaseEditorTest { .clone(); self - .update_cell(&field.id, row_id, content.to_string()) + .update_cell(&field.id, row_id, BoxAny::new(content.to_string())) .await } @@ -258,7 +259,9 @@ impl DatabaseEditorTest { .clone(); let cell_changeset = SelectOptionCellChangeset::from_insert_option_id(option_id); - self.update_cell(&field.id, row_id, cell_changeset).await + self + .update_cell(&field.id, row_id, BoxAny::new(cell_changeset)) + .await } pub async fn import(&self, s: String, format: CSVFormat) -> ImportResult { @@ -315,20 +318,15 @@ impl<'a> TestRowBuilder<'a> { pub fn insert_date_cell( &mut self, - data: i64, + date: i64, time: Option, include_time: Option, field_type: &FieldType, ) -> String { - let value = serde_json::to_string(&DateCellChangeset { - date: Some(data), - time, - include_time, - ..Default::default() - }) - .unwrap(); let date_field = self.field_with_type(field_type); - self.cell_build.insert_text_cell(&date_field.id, value); + self + .cell_build + .insert_date_cell(&date_field.id, date, time, include_time); date_field.id.clone() } diff --git a/frontend/rust-lib/flowy-database2/tests/database/field_test/util.rs b/frontend/rust-lib/flowy-database2/tests/database/field_test/util.rs index 2520aa5e65..1518ba8eec 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/field_test/util.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/field_test/util.rs @@ -3,8 +3,8 @@ use collab_database::views::OrderObjectPosition; use flowy_database2::entities::{CreateFieldParams, FieldType}; use flowy_database2::services::field::{ - type_option_to_pb, DateCellChangeset, DateFormat, DateTypeOption, FieldBuilder, - RichTextTypeOption, SelectOption, SingleSelectTypeOption, TimeFormat, TimestampTypeOption, + type_option_to_pb, DateFormat, DateTypeOption, FieldBuilder, RichTextTypeOption, SelectOption, + SingleSelectTypeOption, TimeFormat, TimestampTypeOption, }; pub fn create_text_field(grid_id: &str) -> (CreateFieldParams, Field) { @@ -103,19 +103,3 @@ pub fn create_timestamp_field(grid_id: &str, field_type: FieldType) -> (CreateFi }; (params, field) } - -// The grid will contains all existing field types and there are three empty rows in this grid. - -pub fn make_date_cell_string(timestamp: i64) -> String { - serde_json::to_string(&DateCellChangeset { - date: Some(timestamp), - time: None, - end_date: None, - end_time: None, - include_time: Some(false), - is_range: Some(false), - clear_flag: None, - reminder_id: Some(String::new()), - }) - .unwrap() -} diff --git a/frontend/rust-lib/flowy-database2/tests/database/group_test/script.rs b/frontend/rust-lib/flowy-database2/tests/database/group_test/script.rs index 2117a984c4..d434f6144b 100644 --- a/frontend/rust-lib/flowy-database2/tests/database/group_test/script.rs +++ b/frontend/rust-lib/flowy-database2/tests/database/group_test/script.rs @@ -214,7 +214,7 @@ impl DatabaseGroupTest { let cell = match field_type { FieldType::URL => insert_url_cell(cell_data, &field), FieldType::DateTime => { - insert_date_cell(cell_data.parse::().unwrap(), Some(true), &field) + insert_date_cell(cell_data.parse::().unwrap(), None, Some(true), &field) }, _ => { panic!("Unsupported group field type");