chore: fix GroupConfigurationRevision deserialize error

frontend/rust-lib/flowy-grid/src/entities/group_entities/configuration.rs

@@ -17,16 +17,12 @@ pub struct TextGroupConfigurationPB {
 pub struct SelectOptionGroupConfigurationPB {
     #[pb(index = 1)]
     hide_empty: bool,
-
-    #[pb(index = 2)]
-    groups: Vec<GroupRecordPB>,
 }
 
 impl std::convert::From<SelectOptionGroupConfigurationRevision> for SelectOptionGroupConfigurationPB {
     fn from(rev: SelectOptionGroupConfigurationRevision) -> Self {
         Self {
             hide_empty: rev.hide_empty,
-            groups: rev.groups.into_iter().map(GroupRecordPB::from).collect(),
         }
     }
 }

frontend/rust-lib/flowy-grid/src/manager.rs

@@ -131,7 +131,6 @@ impl GridManager {
     async fn get_or_create_grid_editor(&self, grid_id: &str) -> FlowyResult<Arc<GridRevisionEditor>> {
         match self.grid_editors.get(grid_id) {
             None => {
-                tracing::trace!("Create grid editor with id: {}", grid_id);
                 let db_pool = self.grid_user.db_pool()?;
                 let editor = self.make_grid_rev_editor(grid_id, db_pool).await?;
 

frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs

@@ -348,30 +348,18 @@ impl GroupConfigurationWriter for GroupConfigurationWriterImpl {
         &self,
         field_id: &str,
         field_type: FieldTypeRevision,
-        configuration_id: &str,
-        content: String,
+        group_configuration: GroupConfigurationRevision,
     ) -> AFFuture<FlowyResult<()>> {
         let user_id = self.user_id.clone();
-        let configuration_id = configuration_id.to_owned();
         let rev_manager = self.rev_manager.clone();
         let view_pad = self.view_pad.clone();
         let field_id = field_id.to_owned();
 
         wrap_future(async move {
-            let is_contained = view_pad.read().await.contains_group(&field_id, &field_type);
-            let changeset = if is_contained {
-                view_pad.write().await.with_mut_group(
-                    &field_id,
-                    &field_type,
-                    &configuration_id,
-                    |group_configuration| {
-                        group_configuration.content = content;
-                    },
-                )?
-            } else {
-                let group_rev = GroupConfigurationRevision::new(field_id.clone(), field_type, content)?;
-                view_pad.write().await.insert_group(&field_id, &field_type, group_rev)?
-            };
+            let changeset = view_pad
+                .write()
+                .await
+                .insert_group(&field_id, &field_type, group_configuration)?;
 
             if let Some(changeset) = changeset {
                 let _ = apply_change(&user_id, rev_manager, changeset).await?;

frontend/rust-lib/flowy-grid/src/services/group/configuration.rs

@@ -1,8 +1,9 @@
 use crate::services::group::{default_group_configuration, Group};
 use flowy_error::{FlowyError, FlowyResult};
 use flowy_grid_data_model::revision::{
-    FieldRevision, FieldTypeRevision, GroupConfigurationContent, GroupConfigurationRevision, GroupRecordRevision,
+    FieldRevision, FieldTypeRevision, GroupConfigurationContentSerde, GroupConfigurationRevision, GroupRecordRevision,
 };
+use std::marker::PhantomData;
 
 use indexmap::IndexMap;
 use lib_infra::future::AFFuture;
@@ -20,14 +21,13 @@ pub trait GroupConfigurationWriter: Send + Sync + 'static {
         &self,
         field_id: &str,
         field_type: FieldTypeRevision,
-        configuration_id: &str,
-        content: String,
+        group_configuration: GroupConfigurationRevision,
     ) -> AFFuture<FlowyResult<()>>;
 }
 
 pub struct GenericGroupConfiguration<C> {
-    pub configuration: C,
-    configuration_id: String,
+    pub configuration: Arc<GroupConfigurationRevision>,
+    configuration_content: PhantomData<C>,
     field_rev: Arc<FieldRevision>,
     groups_map: IndexMap<String, Group>,
     writer: Arc<dyn GroupConfigurationWriter>,
@@ -35,37 +35,32 @@ pub struct GenericGroupConfiguration<C> {
 
 impl<C> GenericGroupConfiguration<C>
 where
-    C: GroupConfigurationContent,
+    C: GroupConfigurationContentSerde,
 {
+    #[tracing::instrument(level = "trace", skip_all, err)]
     pub async fn new(
         field_rev: Arc<FieldRevision>,
         reader: Arc<dyn GroupConfigurationReader>,
         writer: Arc<dyn GroupConfigurationWriter>,
     ) -> FlowyResult<Self> {
-        let configuration_rev = match reader.get_group_configuration(field_rev.clone()).await {
+        let configuration = match reader.get_group_configuration(field_rev.clone()).await {
             None => {
                 let default_group_configuration = default_group_configuration(&field_rev);
                 writer
-                    .save_group_configuration(
-                        &field_rev.id,
-                        field_rev.ty,
-                        &default_group_configuration.id,
-                        default_group_configuration.content.clone(),
-                    )
+                    .save_group_configuration(&field_rev.id, field_rev.ty, default_group_configuration.clone())
                     .await?;
                 Arc::new(default_group_configuration)
             }
             Some(configuration) => configuration,
         };
 
-        let configuration_id = configuration_rev.id.clone();
-        let configuration = C::from_configuration_content(&configuration_rev.content)?;
+        // let configuration = C::from_configuration_content(&configuration_rev.content)?;
         Ok(Self {
-            configuration_id,
             field_rev,
             groups_map: IndexMap::new(),
             writer,
             configuration,
+            configuration_content: PhantomData,
         })
     }
 
@@ -78,11 +73,12 @@ where
     }
 
     pub(crate) async fn merge_groups(&mut self, groups: Vec<Group>) -> FlowyResult<()> {
-        let (group_revs, groups) = merge_groups(self.configuration.get_groups(), groups);
-        self.configuration.set_groups(group_revs);
-        let _ = self.save_configuration().await?;
+        let (group_revs, groups) = merge_groups(&self.configuration.groups, groups);
+        self.mut_configuration(move |configuration| {
+            configuration.groups = group_revs;
+            true
+        })?;
 
-        tracing::trace!("merge new groups: {}", groups.len());
         groups.into_iter().for_each(|group| {
             self.groups_map.insert(group.id.clone(), group);
         });
@@ -91,19 +87,17 @@ where
 
     #[allow(dead_code)]
     pub(crate) async fn hide_group(&mut self, group_id: &str) -> FlowyResult<()> {
-        self.configuration.with_mut_group(group_id, |group_rev| {
+        self.mut_configuration_group(group_id, |group_rev| {
             group_rev.visible = false;
-        });
-        let _ = self.save_configuration().await?;
+        })?;
         Ok(())
     }
 
     #[allow(dead_code)]
     pub(crate) async fn show_group(&mut self, group_id: &str) -> FlowyResult<()> {
-        self.configuration.with_mut_group(group_id, |group_rev| {
+        self.mut_configuration_group(group_id, |group_rev| {
             group_rev.visible = true;
-        });
-        let _ = self.save_configuration().await?;
+        })?;
         Ok(())
     }
 
@@ -123,7 +117,21 @@ where
         match (from_group_index, to_group_index) {
             (Some(from_index), Some(to_index)) => {
                 self.groups_map.swap_indices(from_index, to_index);
-                self.configuration.swap_group(from_group_id, to_group_id);
+
+                self.mut_configuration(|configuration| {
+                    let from_index = configuration
+                        .groups
+                        .iter()
+                        .position(|group| group.group_id == from_group_id);
+                    let to_index = configuration
+                        .groups
+                        .iter()
+                        .position(|group| group.group_id == to_group_id);
+                    if let (Some(from), Some(to)) = (from_index, to_index) {
+                        configuration.groups.swap(from, to);
+                    }
+                    true
+                })?;
                 Ok(())
             }
             _ => Err(FlowyError::out_of_bounds()),
@@ -138,24 +146,54 @@ where
         }
     }
 
-    pub async fn save_configuration(&self) -> FlowyResult<()> {
-        let content = self.configuration.to_configuration_content()?;
-        let _ = self
-            .writer
-            .save_group_configuration(&self.field_rev.id, self.field_rev.ty, &self.configuration_id, content)
-            .await?;
+    pub fn save_configuration(&self) -> FlowyResult<()> {
+        let configuration = (&*self.configuration).clone();
+        let writer = self.writer.clone();
+        let field_id = self.field_rev.id.clone();
+        let field_type = self.field_rev.ty.clone();
+        tokio::spawn(async move {
+            match writer
+                .save_group_configuration(&field_id, field_type, configuration)
+                .await
+            {
+                Ok(_) => {}
+                Err(e) => {
+                    tracing::error!("Save group configuration failed: {}", e);
+                }
+            }
+        });
+
         Ok(())
     }
-}
 
-// impl<T> GroupConfigurationReader for Arc<T>
-// where
-//     T: GroupConfigurationReader,
-// {
-//     fn get_group_configuration(&self, field_rev: Arc<FieldRevision>) -> AFFuture<Arc<GroupConfigurationRevision>> {
-//         (**self).get_group_configuration(field_rev)
-//     }
-// }
+    fn mut_configuration_group(
+        &mut self,
+        group_id: &str,
+        mut_groups_fn: impl Fn(&mut GroupRecordRevision),
+    ) -> FlowyResult<()> {
+        self.mut_configuration(|configuration| {
+            match configuration.groups.iter_mut().find(|group| group.group_id == group_id) {
+                None => false,
+                Some(group_rev) => {
+                    mut_groups_fn(group_rev);
+                    true
+                }
+            }
+        })
+    }
+
+    fn mut_configuration(
+        &mut self,
+        mut_configuration_fn: impl FnOnce(&mut GroupConfigurationRevision) -> bool,
+    ) -> FlowyResult<()> {
+        let configuration = Arc::make_mut(&mut self.configuration);
+        let is_changed = mut_configuration_fn(configuration);
+        if is_changed {
+            let _ = self.save_configuration()?;
+        }
+        Ok(())
+    }
+}
 
 fn merge_groups(old_group_revs: &[GroupRecordRevision], groups: Vec<Group>) -> (Vec<GroupRecordRevision>, Vec<Group>) {
     tracing::trace!("Merge group: old: {}, new: {}", old_group_revs.len(), groups.len());
@@ -172,6 +210,7 @@ fn merge_groups(old_group_revs: &[GroupRecordRevision], groups: Vec<Group>) -> (
         group_map.insert(group.id.clone(), group);
     });
 
+    // Inert
     let mut sorted_groups: Vec<Group> = vec![];
     for group_rev in old_group_revs {
         if let Some(group) = group_map.remove(&group_rev.group_id) {
@@ -184,5 +223,6 @@ fn merge_groups(old_group_revs: &[GroupRecordRevision], groups: Vec<Group>) -> (
         .map(|group| GroupRecordRevision::new(group.id.clone()))
         .collect::<Vec<GroupRecordRevision>>();
 
+    tracing::trace!("group revs: {}, groups: {}", new_group_revs.len(), sorted_groups.len());
     (new_group_revs, sorted_groups)
 }

frontend/rust-lib/flowy-grid/src/services/group/controller.rs

@@ -5,7 +5,7 @@ use crate::services::group::configuration::GenericGroupConfiguration;
 use crate::services::group::entities::Group;
 use flowy_error::FlowyResult;
 use flowy_grid_data_model::revision::{
-    FieldRevision, GroupConfigurationContent, RowChangeset, RowRevision, TypeOptionDataDeserializer,
+    FieldRevision, GroupConfigurationContentSerde, RowChangeset, RowRevision, TypeOptionDataDeserializer,
 };
 
 use std::marker::PhantomData;
@@ -76,7 +76,7 @@ pub struct GenericGroupController<C, T, G, P> {
 
 impl<C, T, G, P> GenericGroupController<C, T, G, P>
 where
-    C: GroupConfigurationContent,
+    C: GroupConfigurationContentSerde,
     T: TypeOptionDataDeserializer,
     G: GroupGenerator<ConfigurationType = GenericGroupConfiguration<C>, TypeOptionType = T>,
 {
@@ -109,7 +109,7 @@ where
 impl<C, T, G, P> GroupControllerSharedOperation for GenericGroupController<C, T, G, P>
 where
     P: CellBytesParser,
-    C: GroupConfigurationContent,
+    C: GroupConfigurationContentSerde,
     Self: GroupAction<CellDataType = P::Object>,
 {
     fn field_id(&self) -> &str {

frontend/rust-lib/flowy-grid/src/services/group/group_service.rs

@@ -71,12 +71,14 @@ impl GroupService {
                             vec![]
                         }
                     };
-
                     drop(write_guard);
                 }
                 Some(groups)
             }
-            Err(_) => Some(vec![]),
+            Err(err) => {
+                tracing::error!("Load group failed: {}", err);
+                Some(vec![])
+            }
         }
     }
 
@@ -183,7 +185,7 @@ impl GroupService {
         }
     }
 
-    #[tracing::instrument(level = "trace", skip_all)]
+    #[tracing::instrument(level = "trace", skip(self, field_rev), err)]
     async fn make_group_controller(
         &self,
         field_type: &FieldType,

shared-lib/flowy-grid-data-model/src/revision/grid_setting_rev.rs

@@ -25,11 +25,11 @@ pub type FilterConfigurationsByFieldId = HashMap<String, Vec<Arc<FilterConfigura
 pub type GroupConfiguration = Configuration<GroupConfigurationRevision>;
 pub type GroupConfigurationsByFieldId = HashMap<String, Vec<Arc<GroupConfigurationRevision>>>;
 
-#[derive(Debug, Clone, Serialize, Deserialize, Default, Eq, PartialEq)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 #[serde(transparent)]
 pub struct Configuration<T>
 where
-    T: Debug + Clone + Default + Eq + PartialEq + serde::Serialize + serde::de::DeserializeOwned + 'static,
+    T: Debug + Clone + Default + serde::Serialize + serde::de::DeserializeOwned + 'static,
 {
     /// Key:    field_id
     /// Value:  this value contains key/value.
@@ -41,7 +41,7 @@ where
 
 impl<T> Configuration<T>
 where
-    T: Debug + Clone + Default + Eq + PartialEq + serde::Serialize + serde::de::DeserializeOwned + 'static,
+    T: Debug + Clone + Default + serde::Serialize + serde::de::DeserializeOwned + 'static,
 {
     pub fn get_mut_objects(&mut self, field_id: &str, field_type: &FieldTypeRevision) -> Option<&mut Vec<Arc<T>>> {
         let value = self
@@ -93,11 +93,11 @@ where
     }
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize, Default, Eq, PartialEq)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 #[serde(transparent)]
 pub struct ObjectIndexMap<T>
 where
-    T: Debug + Clone + Default + Eq + PartialEq + serde::Serialize + serde::de::DeserializeOwned + 'static,
+    T: Debug + Clone + Default + serde::Serialize + serde::de::DeserializeOwned + 'static,
 {
     #[serde(with = "indexmap::serde_seq")]
     pub object_by_field_type: IndexMap<FieldTypeRevision, Vec<Arc<T>>>,
@@ -105,7 +105,7 @@ where
 
 impl<T> ObjectIndexMap<T>
 where
-    T: Debug + Clone + Default + Eq + PartialEq + serde::Serialize + serde::de::DeserializeOwned + 'static,
+    T: Debug + Clone + Default + serde::Serialize + serde::de::DeserializeOwned + 'static,
 {
     pub fn new() -> Self {
         ObjectIndexMap::default()
@@ -114,7 +114,7 @@ where
 
 impl<T> std::ops::Deref for ObjectIndexMap<T>
 where
-    T: Debug + Clone + Default + Eq + PartialEq + serde::Serialize + serde::de::DeserializeOwned + 'static,
+    T: Debug + Clone + Default + serde::Serialize + serde::de::DeserializeOwned + 'static,
 {
     type Target = IndexMap<FieldTypeRevision, Vec<Arc<T>>>;
 
@@ -125,7 +125,7 @@ where
 
 impl<T> std::ops::DerefMut for ObjectIndexMap<T>
 where
-    T: Debug + Clone + Default + Eq + PartialEq + serde::Serialize + serde::de::DeserializeOwned + 'static,
+    T: Debug + Clone + Default + serde::Serialize + serde::de::DeserializeOwned + 'static,
 {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.object_by_field_type

shared-lib/flowy-grid-data-model/src/revision/group_rev.rs

@@ -3,57 +3,32 @@ use serde::{Deserialize, Serialize};
 use serde_json::Error;
 use serde_repr::*;
 
-pub trait GroupConfigurationContent: Sized {
+pub trait GroupConfigurationContentSerde: Sized + Send + Sync {
     fn from_configuration_content(s: &str) -> Result<Self, serde_json::Error>;
 
     fn to_configuration_content(&self) -> Result<String, serde_json::Error>;
-
-    fn get_groups(&self) -> &[GroupRecordRevision] {
-        &[]
-    }
-
-    fn mut_groups(&mut self) -> &mut Vec<GroupRecordRevision> {
-        todo!()
-    }
-
-    fn set_groups(&mut self, _new_groups: Vec<GroupRecordRevision>) {}
-
-    fn swap_group(&mut self, from_group_id: &str, to_group_id: &str) {
-        let from_index = self
-            .get_groups()
-            .iter()
-            .position(|group| group.group_id == from_group_id);
-        let to_index = self.get_groups().iter().position(|group| group.group_id == to_group_id);
-        if let (Some(from), Some(to)) = (from_index, to_index) {
-            self.mut_groups().swap(from, to);
-        }
-    }
-
-    fn with_mut_group<F>(&mut self, _group_id: &str, _f: F)
-    where
-        F: FnOnce(&mut GroupRecordRevision),
-    {
-    }
 }
 
-#[derive(Debug, Clone, Serialize, Deserialize, Default, PartialEq, Eq)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub struct GroupConfigurationRevision {
     pub id: String,
     pub field_id: String,
     pub field_type_rev: FieldTypeRevision,
+    pub groups: Vec<GroupRecordRevision>,
     pub content: String,
 }
 
 impl GroupConfigurationRevision {
     pub fn new<T>(field_id: String, field_type: FieldTypeRevision, content: T) -> Result<Self, serde_json::Error>
     where
-        T: serde::Serialize,
+        T: GroupConfigurationContentSerde,
     {
-        let content = serde_json::to_string(&content)?;
+        let content = content.to_configuration_content()?;
         Ok(Self {
             id: gen_grid_group_id(),
             field_id,
             field_type_rev: field_type,
+            groups: vec![],
             content,
         })
     }
@@ -64,7 +39,7 @@ pub struct TextGroupConfigurationRevision {
     pub hide_empty: bool,
 }
 
-impl GroupConfigurationContent for TextGroupConfigurationRevision {
+impl GroupConfigurationContentSerde for TextGroupConfigurationRevision {
     fn from_configuration_content(s: &str) -> Result<Self, Error> {
         serde_json::from_str(s)
     }
@@ -78,7 +53,7 @@ pub struct NumberGroupConfigurationRevision {
     pub hide_empty: bool,
 }
 
-impl GroupConfigurationContent for NumberGroupConfigurationRevision {
+impl GroupConfigurationContentSerde for NumberGroupConfigurationRevision {
     fn from_configuration_content(s: &str) -> Result<Self, Error> {
         serde_json::from_str(s)
     }
@@ -92,7 +67,7 @@ pub struct UrlGroupConfigurationRevision {
     pub hide_empty: bool,
 }
 
-impl GroupConfigurationContent for UrlGroupConfigurationRevision {
+impl GroupConfigurationContentSerde for UrlGroupConfigurationRevision {
     fn from_configuration_content(s: &str) -> Result<Self, Error> {
         serde_json::from_str(s)
     }
@@ -106,7 +81,7 @@ pub struct CheckboxGroupConfigurationRevision {
     pub hide_empty: bool,
 }
 
-impl GroupConfigurationContent for CheckboxGroupConfigurationRevision {
+impl GroupConfigurationContentSerde for CheckboxGroupConfigurationRevision {
     fn from_configuration_content(s: &str) -> Result<Self, Error> {
         serde_json::from_str(s)
     }
@@ -119,10 +94,9 @@ impl GroupConfigurationContent for CheckboxGroupConfigurationRevision {
 #[derive(Default, Serialize, Deserialize)]
 pub struct SelectOptionGroupConfigurationRevision {
     pub hide_empty: bool,
-    pub groups: Vec<GroupRecordRevision>,
 }
 
-impl GroupConfigurationContent for SelectOptionGroupConfigurationRevision {
+impl GroupConfigurationContentSerde for SelectOptionGroupConfigurationRevision {
     fn from_configuration_content(s: &str) -> Result<Self, Error> {
         serde_json::from_str(s)
     }
@@ -130,31 +104,9 @@ impl GroupConfigurationContent for SelectOptionGroupConfigurationRevision {
     fn to_configuration_content(&self) -> Result<String, Error> {
         serde_json::to_string(self)
     }
-
-    fn get_groups(&self) -> &[GroupRecordRevision] {
-        &self.groups
-    }
-
-    fn mut_groups(&mut self) -> &mut Vec<GroupRecordRevision> {
-        &mut self.groups
-    }
-
-    fn set_groups(&mut self, new_groups: Vec<GroupRecordRevision>) {
-        self.groups = new_groups;
-    }
-
-    fn with_mut_group<F>(&mut self, group_id: &str, f: F)
-    where
-        F: FnOnce(&mut GroupRecordRevision),
-    {
-        match self.groups.iter_mut().find(|group| group.group_id == group_id) {
-            None => {}
-            Some(group) => f(group),
-        }
-    }
 }
 
-#[derive(Clone, Default, Serialize, Deserialize)]
+#[derive(Clone, Debug, Default, Serialize, Deserialize)]
 pub struct GroupRecordRevision {
     pub group_id: String,
 
@@ -179,7 +131,7 @@ pub struct DateGroupConfigurationRevision {
     pub condition: DateCondition,
 }
 
-impl GroupConfigurationContent for DateGroupConfigurationRevision {
+impl GroupConfigurationContentSerde for DateGroupConfigurationRevision {
     fn from_configuration_content(s: &str) -> Result<Self, Error> {
         serde_json::from_str(s)
     }