chore: fix GroupConfigurationRevision deserialize error

appflowy · appflowy · commit ffc6f141fad8 · 2022-08-22T10:06:08.000+08:00
diff --git a/frontend/rust-lib/flowy-grid/src/entities/group_entities/configuration.rs b/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(),
         }
     }
 }
diff --git a/frontend/rust-lib/flowy-grid/src/manager.rs b/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?;
 
diff --git a/frontend/rust-lib/flowy-grid/src/services/grid_view_editor.rs b/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?;
diff --git a/frontend/rust-lib/flowy-grid/src/services/group/configuration.rs b/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,52 +21,46 @@ 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>,
 }
 
 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)
 }
diff --git a/frontend/rust-lib/flowy-grid/src/services/group/controller.rs b/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 {
diff --git a/frontend/rust-lib/flowy-grid/src/services/group/group_service.rs b/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,
diff --git a/shared-lib/flowy-grid-data-model/src/revision/grid_setting_rev.rs b/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,19 +93,19 @@ 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>>>,
 }
 
 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
diff --git a/shared-lib/flowy-grid-data-model/src/revision/group_rev.rs b/shared-lib/flowy-grid-data-model/src/revision/group_rev.rs

Original file line number	Diff line number	Diff line change
`@@ -17,16 +17,12 @@ pub struct TextGroupConfigurationPB {`
`17`	`17`	`pub struct SelectOptionGroupConfigurationPB {`
`18`	`18`	`#[pb(index = 1)]`
`19`	`19`	`hide_empty: bool,`
`20`		`-`
`21`		`- #[pb(index = 2)]`
`22`		`- groups: Vec<GroupRecordPB>,`
`23`	`20`	`}`
`24`	`21`
`25`	`22`	`impl std::convert::From<SelectOptionGroupConfigurationRevision> for SelectOptionGroupConfigurationPB {`
`26`	`23`	`fn from(rev: SelectOptionGroupConfigurationRevision) -> Self {`
`27`	`24`	`Self {`
`28`	`25`	`hide_empty: rev.hide_empty,`
`29`		`- groups: rev.groups.into_iter().map(GroupRecordPB::from).collect(),`
`30`	`26`	`}`
`31`	`27`	`}`
`32`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -71,12 +71,14 @@ impl GroupService {`
`71`	`71`	`vec![]`
`72`	`72`	`}`
`73`	`73`	`};`
`74`		`-`
`75`	`74`	`drop(write_guard);`
`76`	`75`	`}`
`77`	`76`	`Some(groups)`
`78`	`77`	`}`
`79`		`- Err(_) => Some(vec![]),`
	`78`	`+ Err(err) => {`
	`79`	`+ tracing::error!("Load group failed: {}", err);`
	`80`	`+ Some(vec![])`
	`81`	`+ }`
`80`	`82`	`}`
`81`	`83`	`}`
`82`	`84`
`@@ -183,7 +185,7 @@ impl GroupService {`
`183`	`185`	`}`
`184`	`186`	`}`
`185`	`187`
`186`		`- #[tracing::instrument(level = "trace", skip_all)]`
	`188`	`+ #[tracing::instrument(level = "trace", skip(self, field_rev), err)]`
`187`	`189`	`async fn make_group_controller(`
`188`	`190`	`&self,`
`189`	`191`	`field_type: &FieldType,`