From c77329ea11cc519b3158c7edf983b45b572dc360 Mon Sep 17 00:00:00 2001
From: "Nathan.fooo" <86001920+appflowy@users.noreply.github.com>
Date: Wed, 3 Apr 2024 09:07:35 +0800
Subject: [PATCH] chore: encode collab with validate check (#439)

* chore: bump collab

* chore: fix test

* chore: fix test
---
 .github/workflows/integration_test.yml        |  2 --
 Cargo.lock                                    | 10 +++----
 Cargo.toml                                    |  8 +++---
 libs/client-api-test-util/src/test_client.rs  |  6 +++-
 libs/client-api-wasm/Cargo.toml               |  1 -
 libs/client-api/Cargo.toml                    |  1 -
 libs/collab-rt/src/collaborate/group.rs       | 16 ++++-------
 .../src/collaborate/group_persistence.rs      |  5 +---
 libs/collab-rt/src/collaborate/plugin.rs      |  7 ++---
 libs/collab-rt/src/data_validation.rs         | 18 ++----------
 .../src/document/get_started.rs               |  2 +-
 libs/workspace-template/src/lib.rs            |  2 +-
 tests/collab/collab_curd_test.rs              |  1 +
 tests/collab/multi_devices_edit.rs            | 28 ++++++-------------
 tests/collab/storage_test.rs                  |  4 +--
 tests/collab/util.rs                          | 10 +++++--
 tests/collab_snapshot/snapshot_test.rs        |  3 +-
 17 files changed, 50 insertions(+), 74 deletions(-)

diff --git a/.github/workflows/integration_test.yml b/.github/workflows/integration_test.yml
index b4cba707..1bf4e247 100644
--- a/.github/workflows/integration_test.yml
+++ b/.github/workflows/integration_test.yml
@@ -3,8 +3,6 @@ name: AppFlowy-Cloud Integrations
 on:
   push:
     branches: [ main ]
-  pull_request_target:
-    branches: [ main ]
   pull_request:
     branches: [ main ]
 
diff --git a/Cargo.lock b/Cargo.lock
index a9dc1534..cc8554a4 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1326,7 +1326,6 @@ dependencies = [
  "gotrue-entity",
  "governor",
  "mime",
- "mime_guess",
  "parking_lot 0.12.1",
  "prost",
  "reqwest",
@@ -1390,7 +1389,6 @@ dependencies = [
  "client-api",
  "console_error_panic_hook",
  "lazy_static",
- "log",
  "serde",
  "serde_json",
  "tracing",
@@ -1423,7 +1421,7 @@ dependencies = [
 [[package]]
 name = "collab"
 version = "0.1.0"
-source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2#9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2"
+source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=a7a990dfc62a766829d28d2a9bb383840d8146f4#a7a990dfc62a766829d28d2a9bb383840d8146f4"
 dependencies = [
  "anyhow",
  "async-trait",
@@ -1447,7 +1445,7 @@ dependencies = [
 [[package]]
 name = "collab-document"
 version = "0.1.0"
-source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2#9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2"
+source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=a7a990dfc62a766829d28d2a9bb383840d8146f4#a7a990dfc62a766829d28d2a9bb383840d8146f4"
 dependencies = [
  "anyhow",
  "collab",
@@ -1466,7 +1464,7 @@ dependencies = [
 [[package]]
 name = "collab-entity"
 version = "0.1.0"
-source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2#9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2"
+source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=a7a990dfc62a766829d28d2a9bb383840d8146f4#a7a990dfc62a766829d28d2a9bb383840d8146f4"
 dependencies = [
  "anyhow",
  "bytes",
@@ -1481,7 +1479,7 @@ dependencies = [
 [[package]]
 name = "collab-folder"
 version = "0.1.0"
-source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2#9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2"
+source = "git+https://github.com/AppFlowy-IO/AppFlowy-Collab?rev=a7a990dfc62a766829d28d2a9bb383840d8146f4#a7a990dfc62a766829d28d2a9bb383840d8146f4"
 dependencies = [
  "anyhow",
  "chrono",
diff --git a/Cargo.toml b/Cargo.toml
index 1ec29bc2..d1262953 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -199,10 +199,10 @@ debug = true
 # will be removed when using yrs 0.18.2 that expose pendings
 yrs = { git = "https://github.com/appflowy/y-crdt", rev = "3f25bb510ca5274e7657d3713fbed41fb46b4487" }
 
-collab = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2" }
-collab-entity = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2" }
-collab-folder = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2" }
-collab-document = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "9e519d46bb8c4c5097d8c9dbc8f77707f8041ee2" }
+collab = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "a7a990dfc62a766829d28d2a9bb383840d8146f4" }
+collab-entity = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "a7a990dfc62a766829d28d2a9bb383840d8146f4" }
+collab-folder = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "a7a990dfc62a766829d28d2a9bb383840d8146f4" }
+collab-document = { git = "https://github.com/AppFlowy-IO/AppFlowy-Collab", rev = "a7a990dfc62a766829d28d2a9bb383840d8146f4" }
 
 [features]
 ai_enable = []
\ No newline at end of file
diff --git a/libs/client-api-test-util/src/test_client.rs b/libs/client-api-test-util/src/test_client.rs
index 39c5a2d8..a00a847d 100644
--- a/libs/client-api-test-util/src/test_client.rs
+++ b/libs/client-api-test-util/src/test_client.rs
@@ -522,7 +522,11 @@ impl TestClient {
       ),
     };
 
-    let encoded_collab_v1 = collab.encode_collab_v1().encode_to_bytes().unwrap();
+    let encoded_collab_v1 = collab
+      .encode_collab_v1(|collab| collab_type.validate(collab))
+      .unwrap()
+      .encode_to_bytes()
+      .unwrap();
     self
       .api_client
       .create_collab(CreateCollabParams {
diff --git a/libs/client-api-wasm/Cargo.toml b/libs/client-api-wasm/Cargo.toml
index c5fb6e79..eb1e98c9 100644
--- a/libs/client-api-wasm/Cargo.toml
+++ b/libs/client-api-wasm/Cargo.toml
@@ -20,7 +20,6 @@ wasm-bindgen = "0.2.84"
 # all the `std::fmt` and `std::panicking` infrastructure, so isn't great for
 # code size when deploying.
 console_error_panic_hook = { version = "0.1.7", optional = true }
-log = "0.4.20"
 serde = "1.0.197"
 serde_json = "1.0.64"
 client-api = { path = "../client-api" }
diff --git a/libs/client-api/Cargo.toml b/libs/client-api/Cargo.toml
index accbcf63..193648b8 100644
--- a/libs/client-api/Cargo.toml
+++ b/libs/client-api/Cargo.toml
@@ -22,7 +22,6 @@ futures-util = "0.3.30"
 futures-core = "0.3.30"
 parking_lot = "0.12.1"
 brotli = "3.4.0"
-mime_guess = "2.0.4"
 async-trait = { version = "0.1.77" }
 prost = "0.12.3"
 bincode = "1.3.3"
diff --git a/libs/collab-rt/src/collaborate/group.rs b/libs/collab-rt/src/collaborate/group.rs
index c974d174..80804504 100644
--- a/libs/collab-rt/src/collaborate/group.rs
+++ b/libs/collab-rt/src/collaborate/group.rs
@@ -4,21 +4,17 @@ use collab::preclude::Collab;
 use collab_entity::CollabType;
 use dashmap::DashMap;
 
-use std::rc::Rc;
-use std::sync::atomic::{AtomicI64, AtomicU32, Ordering};
-use std::sync::Arc;
-
 use crate::collaborate::group_broadcast::{CollabBroadcast, Subscription};
-use crate::metrics::CollabMetricsCalculate;
-
 use crate::collaborate::group_persistence::GroupPersistence;
-use crate::data_validation::validate_collab;
 use crate::error::RealtimeError;
-
+use crate::metrics::CollabMetricsCalculate;
 use collab_rt_entity::user::RealtimeUser;
 use collab_rt_entity::CollabMessage;
 use collab_rt_entity::MessageByObjectId;
 use database::collab::CollabStorage;
+use std::rc::Rc;
+use std::sync::atomic::{AtomicI64, AtomicU32, Ordering};
+use std::sync::Arc;
 
 use futures_util::{SinkExt, StreamExt};
 use tokio::sync::{mpsc, Mutex};
@@ -92,8 +88,8 @@ impl CollabGroup {
 
   pub async fn encode_collab(&self) -> Result<EncodedCollab, RealtimeError> {
     let lock_guard = self.collab.lock().await;
-    validate_collab(&lock_guard, &self.collab_type)?;
-    let encode_collab = lock_guard.try_encode_collab_v1()?;
+    let encode_collab =
+      lock_guard.try_encode_collab_v1(|collab| self.collab_type.validate(collab))?;
     Ok(encode_collab)
   }
 
diff --git a/libs/collab-rt/src/collaborate/group_persistence.rs b/libs/collab-rt/src/collaborate/group_persistence.rs
index 81e04a32..09c004e4 100644
--- a/libs/collab-rt/src/collaborate/group_persistence.rs
+++ b/libs/collab-rt/src/collaborate/group_persistence.rs
@@ -1,5 +1,4 @@
 use crate::collaborate::group::EditState;
-use crate::data_validation::validate_collab;
 
 use anyhow::anyhow;
 use app_error::AppError;
@@ -146,10 +145,8 @@ fn get_encode_collab(
   collab: &Collab,
   collab_type: &CollabType,
 ) -> Result<CollabParams, AppError> {
-  validate_collab(collab, collab_type).map_err(|err| AppError::NoRequiredData(err.to_string()))?;
-
   let result = collab
-    .try_encode_collab_v1()
+    .try_encode_collab_v1(|collab| collab_type.validate(collab))
     .map_err(|err| AppError::Internal(anyhow!("fail to encode collab to bytes: {:?}", err)))?
     .encode_to_bytes();
 
diff --git a/libs/collab-rt/src/collaborate/plugin.rs b/libs/collab-rt/src/collaborate/plugin.rs
index f33cfe21..f257a4ce 100644
--- a/libs/collab-rt/src/collaborate/plugin.rs
+++ b/libs/collab-rt/src/collaborate/plugin.rs
@@ -1,4 +1,3 @@
-use crate::data_validation::validate_collab;
 use crate::error::RealtimeError;
 use crate::RealtimeAccessControl;
 use app_error::AppError;
@@ -176,9 +175,9 @@ where
         vec![],
         false,
       ) {
-        if validate_collab(&collab, collab_type).is_ok() {
-          return Some(encoded_collab);
-        }
+        // TODO(nathan): this check is not necessary, can be removed in the future.
+        collab_type.validate(&collab).ok()?;
+        return Some(encoded_collab);
       }
     }
   }
diff --git a/libs/collab-rt/src/data_validation.rs b/libs/collab-rt/src/data_validation.rs
index 4073ece6..07d891f5 100644
--- a/libs/collab-rt/src/data_validation.rs
+++ b/libs/collab-rt/src/data_validation.rs
@@ -22,20 +22,8 @@ pub fn validate_encode_collab(
   )
   .map_err(|err| RealtimeError::Internal(err.into()))?;
 
-  validate_collab(&collab, collab_type)
-}
-
-pub fn validate_collab(collab: &Collab, collab_type: &CollabType) -> Result<(), RealtimeError> {
-  match collab_type {
-    CollabType::Document => collab_document::document::Document::validate(collab)
-      .map_err(|err| RealtimeError::NoRequiredCollabData(err.to_string()))?,
-    CollabType::Database => {},
-    CollabType::Folder => collab_folder::Folder::validate(collab)
-      .map(|_| ())
-      .map_err(|err| RealtimeError::NoRequiredCollabData(err.to_string()))?,
-    CollabType::DatabaseRow => {},
-    _ => {},
-  }
-
+  collab_type
+    .validate(&collab)
+    .map_err(|err| RealtimeError::NoRequiredCollabData(err.to_string()))?;
   Ok(())
 }
diff --git a/libs/workspace-template/src/document/get_started.rs b/libs/workspace-template/src/document/get_started.rs
index 89dde5f3..87c3ae24 100644
--- a/libs/workspace-template/src/document/get_started.rs
+++ b/libs/workspace-template/src/document/get_started.rs
@@ -47,7 +47,7 @@ impl WorkspaceTemplate for GetStartedDocumentTemplate {
         false,
       ));
       let document = Document::create_with_data(collab, document_data)?;
-      let data = document.get_collab().encode_collab_v1();
+      let data = document.encode_collab()?;
       Ok::<_, anyhow::Error>(TemplateData {
         object_id: view_id,
         object_type: CollabType::Document,
diff --git a/libs/workspace-template/src/lib.rs b/libs/workspace-template/src/lib.rs
index ddbe747b..47d03956 100644
--- a/libs/workspace-template/src/lib.rs
+++ b/libs/workspace-template/src/lib.rs
@@ -125,7 +125,7 @@ impl WorkspaceTemplateBuilder {
         false,
       ));
       let folder = Folder::create(uid, collab, None, folder_data);
-      let data = folder.encode_collab_v1();
+      let data = folder.encode_collab_v1()?;
       Ok::<_, anyhow::Error>(TemplateData {
         object_id: workspace_id,
         object_type: CollabType::Folder,
diff --git a/tests/collab/collab_curd_test.rs b/tests/collab/collab_curd_test.rs
index 8551c99d..ea590b46 100644
--- a/tests/collab/collab_curd_test.rs
+++ b/tests/collab/collab_curd_test.rs
@@ -90,6 +90,7 @@ async fn create_collab_params_compatibility_serde_test() {
   // This test is to make sure that the CreateCollabParams is compatible with the old InsertCollabParams
   let object_id = uuid::Uuid::new_v4().to_string();
   let encoded_collab_v1 = default_document_collab_data(&object_id)
+    .unwrap()
     .encode_to_bytes()
     .unwrap();
 
diff --git a/tests/collab/multi_devices_edit.rs b/tests/collab/multi_devices_edit.rs
index c7ce00b3..00a19966 100644
--- a/tests/collab/multi_devices_edit.rs
+++ b/tests/collab/multi_devices_edit.rs
@@ -97,11 +97,13 @@ async fn same_client_with_diff_devices_edit_same_collab_test() {
   client_2
     .open_collab(&workspace_id, &object_id, collab_type.clone())
     .await;
-  sleep(Duration::from_millis(1000)).await;
-
+  client_2
+    .wait_object_sync_complete(&object_id)
+    .await
+    .unwrap();
   trace!("client 2 disconnect: {:?}", client_2.device_id);
   client_2.disconnect().await;
-  sleep(Duration::from_millis(1000)).await;
+  sleep(Duration::from_millis(2000)).await;
 
   client_2
     .collabs
@@ -115,22 +117,10 @@ async fn same_client_with_diff_devices_edit_same_collab_test() {
   let expected_json = json!({
     "name": "workspace2"
   });
-  assert_client_collab_within_secs(
-    &mut client_1,
-    &object_id,
-    "name",
-    expected_json.clone(),
-    120,
-  )
-  .await;
-  assert_client_collab_within_secs(
-    &mut client_2,
-    &object_id,
-    "name",
-    expected_json.clone(),
-    120,
-  )
-  .await;
+  assert_client_collab_within_secs(&mut client_1, &object_id, "name", expected_json.clone(), 60)
+    .await;
+  assert_client_collab_within_secs(&mut client_2, &object_id, "name", expected_json.clone(), 60)
+    .await;
 }
 
 #[tokio::test]
diff --git a/tests/collab/storage_test.rs b/tests/collab/storage_test.rs
index 045029d3..ede81269 100644
--- a/tests/collab/storage_test.rs
+++ b/tests/collab/storage_test.rs
@@ -99,11 +99,11 @@ async fn success_part_batch_get_collab_test() {
     },
     QueryCollab {
       object_id: Uuid::new_v4().to_string(),
-      collab_type: CollabType::Folder,
+      collab_type: CollabType::Empty,
     },
     QueryCollab {
       object_id: Uuid::new_v4().to_string(),
-      collab_type: CollabType::Database,
+      collab_type: CollabType::Empty,
     },
   ];
 
diff --git a/tests/collab/util.rs b/tests/collab/util.rs
index 930b6e68..8a34382d 100644
--- a/tests/collab/util.rs
+++ b/tests/collab/util.rs
@@ -29,13 +29,19 @@ pub fn generate_random_string(len: usize) -> String {
 pub fn make_big_collab_doc_state(object_id: &str, key: &str, value: String) -> Vec<u8> {
   let collab = Collab::new_with_origin(CollabOrigin::Empty, object_id, vec![], false);
   collab.insert(key, value);
-  collab.encode_collab_v1().doc_state.to_vec()
+  collab
+    .encode_collab_v1(|_| Ok::<(), anyhow::Error>(()))
+    .unwrap()
+    .doc_state
+    .to_vec()
 }
 
 pub fn test_encode_collab_v1(object_id: &str, key: &str, value: &str) -> EncodedCollab {
   let collab = Collab::new_with_origin(CollabOrigin::Empty, object_id, vec![], false);
   collab.insert(key, value);
-  collab.encode_collab_v1()
+  collab
+    .encode_collab_v1(|_| Ok::<(), anyhow::Error>(()))
+    .unwrap()
 }
 
 #[allow(dead_code)]
diff --git a/tests/collab_snapshot/snapshot_test.rs b/tests/collab_snapshot/snapshot_test.rs
index c3ea42ea..b80980fd 100644
--- a/tests/collab_snapshot/snapshot_test.rs
+++ b/tests/collab_snapshot/snapshot_test.rs
@@ -1,3 +1,4 @@
+use anyhow::Error;
 use assert_json_diff::assert_json_eq;
 use collab::core::collab::{DocStateSource, MutexCollab};
 use collab::core::collab_plugin::EncodedCollab;
@@ -144,7 +145,7 @@ fn test_collab_data(uid: i64, oid: &str) -> (EncodedCollab, Value) {
     collab.insert_with_txn(txn, "2", "c");
   });
   (
-    collab.encode_collab_v1(),
+    collab.encode_collab_v1(|_| Ok::<(), Error>(())).unwrap(),
     json!({
       "0": "a",
       "1": "b",