refactor: add Tenant to ShareConsumer (#15142)

drmingdrmer · web-flow · commit cab23ee77031 · 2024-03-31T22:50:20.000+08:00
diff --git a/src/meta/api/src/share_api_impl.rs b/src/meta/api/src/share_api_impl.rs
@@ -327,7 +327,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
             for account in req.accounts.iter() {
                 if !share_meta.has_account(account) {
                     add_share_account_keys.push(ShareConsumer {
-                        tenant: account.clone(),
+                        tenant: Tenant::new_or_err(account, "add_share_tenants")?,
                         share_id,
                     });
                 }
@@ -358,7 +358,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
                     condition.push(txn_cond_seq(share_account_key, Eq, 0));
 
                     let share_account_meta = ShareAccountMeta::new(
-                        share_account_key.tenant.clone(),
+                        share_account_key.tenant.name().to_string(),
                         share_id,
                         req.share_on,
                     );
@@ -368,7 +368,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
                         serialize_struct(&share_account_meta)?,
                     )); /* (account, share_id) -> share_account_meta */
 
-                    share_meta.add_account(share_account_key.tenant.clone());
+                    share_meta.add_account(share_account_key.tenant.name().to_string());
                 }
                 if_then.push(txn_op_put(&id_key, serialize_struct(&share_meta)?)); /* (share_id) -> share_meta */
 
@@ -438,7 +438,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
                 }
                 if share_meta.has_account(account) {
                     let share_account_key = ShareConsumer {
-                        tenant: account.clone(),
+                        tenant: Tenant::new_or_err(account, "remove_share_tenants")?,
                         share_id,
                     };
 
@@ -485,7 +485,7 @@ impl<KV: kvapi::KVApi<Error = MetaError>> ShareApi for KV {
 
                     if_then.push(txn_op_del(&share_account_key_and_seq.0)); // del (account, share_id)
 
-                    share_meta.del_account(&share_account_key_and_seq.0.tenant);
+                    share_meta.del_account(share_account_key_and_seq.0.tenant.name());
                 }
                 if_then.push(txn_op_put(&id_key, serialize_struct(&share_meta)?)); /* (share_id) -> share_meta */
 
@@ -1358,7 +1358,7 @@ async fn get_outbound_share_tenants_by_name(
     let mut accounts = vec![];
     for account in share_meta.get_accounts() {
         let share_account_key = ShareConsumer {
-            tenant: account.clone(),
+            tenant: Tenant::new_or_err(&account, "get_outbound_share_tenants_by_name")?,
             share_id,
         };
 
@@ -1652,7 +1652,7 @@ async fn drop_accounts_granted_from_share(
     // get all accounts seq from share_meta
     for account in share_meta.get_accounts() {
         let share_account_key = ShareConsumer {
-            tenant: account.clone(),
+            tenant: Tenant::new_or_err(&account, "drop_accounts_granted_from_share")?,
             share_id,
         };
         let ret = get_share_account_meta_or_err(
diff --git a/src/meta/api/src/share_api_test_suite.rs b/src/meta/api/src/share_api_test_suite.rs
@@ -82,7 +82,7 @@ async fn is_all_share_data_removed(
 
     for account in share_meta.get_accounts() {
         let share_account_key = ShareConsumer {
-            tenant: account.clone(),
+            tenant: Tenant::new_or_err(account, "is_all_share_data_removed")?,
             share_id,
         };
         let res = get_share_account_meta_or_err(kv_api, &share_account_key, "").await;
@@ -581,7 +581,7 @@ impl ShareApiTestSuite {
 
             // get and check share account meta
             let share_account_name = ShareConsumer {
-                tenant: account.to_string(),
+                tenant: Tenant::new_or_err(account, "share_add_remove_account")?,
                 share_id,
             };
             let (_share_account_meta_seq, share_account_meta) =
@@ -707,7 +707,7 @@ impl ShareApiTestSuite {
 
             // check share account meta has been removed
             let share_account_name = ShareConsumer {
-                tenant: account2.to_string(),
+                tenant: Tenant::new_or_err(account2, "share_add_remove_account")?,
                 share_id,
             };
             let res = get_share_account_meta_or_err(mt.as_kv_api(), &share_account_name, "").await;
@@ -730,7 +730,7 @@ impl ShareApiTestSuite {
 
             // check share account meta has been removed
             let share_account_name = ShareConsumer {
-                tenant: account.to_string(),
+                tenant: Tenant::new_or_err(account, "share_add_remove_account")?,
                 share_id,
             };
             let res = get_share_account_meta_or_err(mt.as_kv_api(), &share_account_name, "").await;
diff --git a/src/meta/api/src/util.rs b/src/meta/api/src/util.rs
@@ -638,7 +638,7 @@ fn share_account_meta_has_to_exist(
 
         Err(KVAppError::AppError(AppError::UnknownShareAccounts(
             UnknownShareAccounts::new(
-                &[name_key.tenant.clone()],
+                &[name_key.tenant.name().to_string()],
                 name_key.share_id,
                 format!("{}: {}", msg, name_key),
             ),
diff --git a/src/meta/app/src/share/share.rs b/src/meta/app/src/share/share.rs
@@ -31,6 +31,7 @@ use crate::schema::TableInfo;
 use crate::schema::TableMeta;
 use crate::share::ShareEndpointIdent;
 use crate::tenant::Tenant;
+use crate::tenant::ToTenant;
 
 // serde is required by `DatabaseType`
 /// A share that is created by `tenant` and is named with `share_name`.
@@ -48,15 +49,24 @@ impl Display for ShareNameIdent {
 
 /// The share consuming key describes that the `tenant`, who is a consumer of a shared object,
 /// which is created by another tenant and is identified by `share_id`.
-#[derive(Clone, Debug, Default, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 pub struct ShareConsumer {
-    pub tenant: String,
+    pub tenant: Tenant,
     pub share_id: u64,
 }
 
+impl ShareConsumer {
+    pub fn new(tenant: impl ToTenant, share_id: u64) -> Self {
+        Self {
+            tenant: tenant.to_tenant(),
+            share_id,
+        }
+    }
+}
+
 impl Display for ShareConsumer {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "'{}'/'{}'", self.tenant, self.share_id)
+        write!(f, "'{}'/'{}'", self.tenant.name(), self.share_id)
     }
 }
 
@@ -608,7 +618,7 @@ impl ShareMeta {
         self.accounts.insert(account);
     }
 
-    pub fn del_account(&mut self, account: &String) {
+    pub fn del_account(&mut self, account: &str) {
         self.accounts.remove(account);
     }
 
@@ -885,33 +895,32 @@ mod kvapi_key_impl {
 
         /// It belongs to a tenant
         fn parent(&self) -> Option<String> {
-            Some(Tenant::new(&self.tenant).to_string_key())
+            Some(kvapi::Key::to_string_key(&self.tenant))
         }
 
         fn to_string_key(&self) -> String {
             if self.share_id != 0 {
                 kvapi::KeyBuilder::new_prefixed(Self::PREFIX)
-                    .push_str(&self.tenant)
+                    .push_str(self.tenant.name())
                     .push_u64(self.share_id)
                     .done()
             } else {
                 kvapi::KeyBuilder::new_prefixed(Self::PREFIX)
-                    .push_str(&self.tenant)
+                    .push_str(self.tenant.name())
                     .done()
             }
         }
 
         fn from_str_key(s: &str) -> Result<Self, kvapi::KeyError> {
             let mut p = kvapi::KeyParser::new_prefixed(s, Self::PREFIX)?;
 
-            let account = p.next_str()?;
+            let tenant = p.next_nonempty()?;
             let share_id = p.next_u64()?;
             p.done()?;
 
-            Ok(ShareConsumer {
-                tenant: account,
-                share_id,
-            })
+            let tenant = Tenant::new_nonempty(tenant);
+
+            Ok(ShareConsumer { tenant, share_id })
         }
     }
 
diff --git a/src/meta/app/src/tenant/tenant.rs b/src/meta/app/src/tenant/tenant.rs
@@ -28,13 +28,6 @@ pub struct Tenant {
 }
 
 impl Tenant {
-    // #[deprecated]
-    pub fn new(tenant: impl ToString) -> Self {
-        Self {
-            tenant: tenant.to_string(),
-        }
-    }
-
     pub fn new_or_err(tenant: impl ToString, ctx: impl Display) -> Result<Self, TenantIsEmpty> {
         let non_empty =
             NonEmptyString::new(tenant.to_string()).map_err(|_e| TenantIsEmpty::new(ctx))?;
