refactor: use SettingIdent to build string key to access settings

src/meta/api/src/kv_pb_api/mod.rs

@@ -180,6 +180,7 @@ pub trait KVPbApi: KVApi {
         K::ValueType: FromToProto,
     {
         let prefix = prefix.to_string_key();
+        let prefix = format!("{prefix}/");
         async move {
             let strm = self
                 .list_kv(&prefix)

src/meta/app/src/principal/mod.rs

@@ -31,6 +31,7 @@ mod user_info;
 mod user_privilege;
 mod user_quota;
 mod user_setting;
+mod user_setting_ident;
 mod user_stage;
 mod user_stage_file_ident;
 mod user_stage_ident;
@@ -69,6 +70,7 @@ pub use user_privilege::UserPrivilegeType;
 pub use user_quota::UserQuota;
 pub use user_setting::UserSetting;
 pub use user_setting::UserSettingValue;
+pub use user_setting_ident::SettingIdent;
 pub use user_stage::*;
 pub use user_stage_file_ident::StageFileIdent;
 pub use user_stage_ident::StageIdent;

src/meta/app/src/principal/user_setting_ident.rs

@@ -0,0 +1,93 @@
+// Copyright 2021 Datafuse Labs
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use crate::tenant::Tenant;
+
+/// Define the meta-service key for a user setting.
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct SettingIdent {
+    pub tenant: Tenant,
+    pub name: String,
+}
+
+impl SettingIdent {
+    pub fn new(tenant: Tenant, name: impl ToString) -> Self {
+        Self {
+            tenant,
+            name: name.to_string(),
+        }
+    }
+}
+
+mod kvapi_key_impl {
+    use databend_common_meta_kvapi::kvapi;
+    use databend_common_meta_kvapi::kvapi::KeyError;
+
+    use crate::principal::user_setting_ident::SettingIdent;
+    use crate::principal::UserSetting;
+    use crate::tenant::Tenant;
+    use crate::KeyWithTenant;
+
+    impl kvapi::Key for SettingIdent {
+        const PREFIX: &'static str = "__fd_settings";
+        type ValueType = UserSetting;
+
+        fn parent(&self) -> Option<String> {
+            Some(self.tenant.to_string_key())
+        }
+
+        fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
+            b.push_str(self.tenant_name()).push_str(&self.name)
+        }
+
+        fn decode_key(p: &mut kvapi::KeyParser) -> Result<Self, KeyError> {
+            let tenant = p.next_nonempty()?;
+            let name = p.next_str()?;
+
+            Ok(SettingIdent::new(Tenant::new_nonempty(tenant), name))
+        }
+    }
+
+    impl KeyWithTenant for SettingIdent {
+        fn tenant(&self) -> &Tenant {
+            &self.tenant
+        }
+    }
+
+    impl kvapi::Value for UserSetting {
+        fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
+            []
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use databend_common_meta_kvapi::kvapi::Key;
+
+    use crate::principal::user_setting_ident::SettingIdent;
+    use crate::tenant::Tenant;
+
+    #[test]
+    fn test_setting_ident() {
+        let tenant = Tenant::new("tenant1");
+        let ident = SettingIdent::new(tenant.clone(), "test");
+        assert_eq!(tenant, ident.tenant);
+        assert_eq!("test", ident.name);
+        assert_eq!("__fd_settings/tenant1/test", ident.to_string_key());
+
+        let got = SettingIdent::from_str_key(&ident.to_string_key()).unwrap();
+        assert_eq!(ident, got);
+    }
+}

src/query/management/src/setting/setting_mgr.rs

@@ -16,8 +16,12 @@ use std::sync::Arc;
 
 use databend_common_exception::ErrorCode;
 use databend_common_exception::Result;
+use databend_common_meta_app::principal::SettingIdent;
 use databend_common_meta_app::principal::UserSetting;
+use databend_common_meta_app::tenant::Tenant;
 use databend_common_meta_kvapi::kvapi;
+use databend_common_meta_kvapi::kvapi::DirName;
+use databend_common_meta_kvapi::kvapi::Key;
 use databend_common_meta_kvapi::kvapi::UpsertKVReq;
 use databend_common_meta_types::IntoSeqV;
 use databend_common_meta_types::MatchSeq;
@@ -30,11 +34,9 @@ use databend_common_meta_types::SeqValue;
 
 use crate::setting::SettingApi;
 
-static USER_SETTING_API_KEY_PREFIX: &str = "__fd_settings";
-
 pub struct SettingMgr {
     kv_api: Arc<dyn kvapi::KVApi<Error = MetaError>>,
-    setting_prefix: String,
+    tenant: Tenant,
 }
 
 impl SettingMgr {
@@ -44,11 +46,24 @@ impl SettingMgr {
     ) -> Self {
         SettingMgr {
             kv_api,
-            setting_prefix: format!("{}/{}", USER_SETTING_API_KEY_PREFIX, tenant.as_str()),
+            tenant: Tenant::new_nonempty(tenant.clone()),
         }
     }
+
+    fn setting_ident(&self, name: &str) -> SettingIdent {
+        SettingIdent::new(self.tenant.clone(), name)
+    }
+
+    fn setting_key(&self, name: &str) -> String {
+        self.setting_ident(name).to_string_key()
+    }
+
+    fn setting_prefix(&self) -> String {
+        self.setting_ident("").to_string_key()
+    }
 }
 
+// TODO: do not use json for setting value
 #[async_trait::async_trait]
 impl SettingApi for SettingMgr {
     #[async_backtrace::framed]
@@ -57,7 +72,7 @@ impl SettingApi for SettingMgr {
         // Upsert.
         let seq = MatchSeq::GE(0);
         let val = Operation::Update(serde_json::to_vec(&setting)?);
-        let key = format!("{}/{}", self.setting_prefix, setting.name);
+        let key = self.setting_key(&setting.name);
         let upsert = self
             .kv_api
             .upsert_kv(UpsertKVReq::new(&key, seq, val, None));
@@ -70,7 +85,8 @@ impl SettingApi for SettingMgr {
     #[async_backtrace::framed]
     #[minitrace::trace]
     async fn get_settings(&self) -> Result<Vec<UserSetting>> {
-        let values = self.kv_api.prefix_list_kv(&self.setting_prefix).await?;
+        let prefix = self.setting_prefix();
+        let values = self.kv_api.prefix_list_kv(&prefix).await?;
 
         let mut settings = Vec::with_capacity(values.len());
         for (_, value) in values {
@@ -83,10 +99,9 @@ impl SettingApi for SettingMgr {
     #[async_backtrace::framed]
     #[minitrace::trace]
     async fn get_setting(&self, name: &str, seq: MatchSeq) -> Result<SeqV<UserSetting>> {
-        let key = format!("{}/{}", self.setting_prefix, name);
-        let kv_api = self.kv_api.clone();
-        let get_kv = async move { kv_api.get_kv(&key).await };
-        let res = get_kv.await?;
+        let key = self.setting_key(name);
+        let res = self.kv_api.get_kv(&key).await?;
+
         let seq_value = res.ok_or_else(|| {
             ErrorCode::UnknownVariable(format!("Setting '{}' does not exist.", name))
         })?;
@@ -103,14 +118,12 @@ impl SettingApi for SettingMgr {
     #[async_backtrace::framed]
     #[minitrace::trace]
     async fn try_drop_setting(&self, name: &str, seq: MatchSeq) -> Result<()> {
-        let key = format!("{}/{}", self.setting_prefix, name);
-        let kv_api = self.kv_api.clone();
-        let upsert_kv = async move {
-            kv_api
-                .upsert_kv(UpsertKVReq::new(&key, seq, Operation::Delete, None))
-                .await
-        };
-        upsert_kv.await?;
+        let key = self.setting_key(name);
+        let _res = self
+            .kv_api
+            .upsert_kv(UpsertKVReq::new(&key, seq, Operation::Delete, None))
+            .await?;
+
         Ok(())
     }
 }