refactor(query): normalize role mgr api boundaries

  - RoleApi returns MetaError/MetaTxnError plus ExistError/UnknownError
  - Make get_role return Option instead of raising ErrorCode
  - Simplify update_role_with to internal read-modify-write by seq
  - Map ExistError/UnknownError to ErrorCode in role_ident
  - Update callers to convert meta errors via meta_service_error

Author: TCeason

src/meta/app/src/principal/role_ident.rs

@@ -36,10 +36,13 @@ impl RoleIdentRaw {
 
 mod kvapi_impl {
 
+    use databend_common_exception::ErrorCode;
     use databend_meta_kvapi::kvapi;
 
     use crate::principal::RoleIdent;
     use crate::principal::RoleInfo;
+    use crate::tenant_key::errors::ExistError;
+    use crate::tenant_key::errors::UnknownError;
     use crate::tenant_key::resource::TenantResource;
 
     pub struct Resource;
@@ -58,9 +61,17 @@ mod kvapi_impl {
         }
     }
 
-    // // Use these error types to replace usage of ErrorCode if possible.
-    // impl From<ExistError<Resource>> for ErrorCode {
-    // impl From<UnknownError<Resource>> for ErrorCode {
+    impl From<ExistError<Resource>> for ErrorCode {
+        fn from(err: ExistError<Resource>) -> Self {
+            ErrorCode::RoleAlreadyExists(err.to_string())
+        }
+    }
+
+    impl From<UnknownError<Resource>> for ErrorCode {
+        fn from(err: UnknownError<Resource>) -> Self {
+            ErrorCode::UnknownRole(err.to_string())
+        }
+    }
 }
 
 #[cfg(test)]

src/query/management/src/role/role_api.rs

@@ -12,57 +12,60 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use databend_common_exception::Result;
+use databend_common_meta_api::meta_txn_error::MetaTxnError;
 use databend_common_meta_app::principal::OwnershipInfo;
 use databend_common_meta_app::principal::OwnershipObject;
 use databend_common_meta_app::principal::RoleInfo;
 use databend_common_meta_app::principal::role_ident;
 use databend_common_meta_app::tenant_key::errors::ExistError;
+use databend_common_meta_app::tenant_key::errors::UnknownError;
 use databend_meta_kvapi::kvapi::ListKVReply;
-use databend_meta_types::MatchSeq;
 use databend_meta_types::MetaError;
 use databend_meta_types::SeqV;
 
 #[async_trait::async_trait]
 pub trait RoleApi: Sync + Send {
-    async fn add_role(
+    async fn create_role(
         &self,
         role_info: RoleInfo,
         can_replace: bool,
-    ) -> std::result::Result<std::result::Result<(), ExistError<role_ident::Resource>>, MetaError>;
+    ) -> Result<Result<(), ExistError<role_ident::Resource>>, MetaError>;
 
-    #[allow(clippy::ptr_arg)]
-    async fn get_role(&self, role: &str, seq: MatchSeq) -> Result<SeqV<RoleInfo>>;
+    async fn get_role(&self, role: &str) -> Result<Option<SeqV<RoleInfo>>, MetaError>;
 
     /// get all roles that store in meta
-    async fn get_meta_roles(&self) -> Result<Vec<SeqV<RoleInfo>>>;
+    async fn get_meta_roles(&self) -> Result<Vec<SeqV<RoleInfo>>, MetaError>;
 
-    async fn get_raw_meta_roles(&self) -> Result<ListKVReply>;
+    async fn get_raw_meta_roles(&self) -> Result<ListKVReply, MetaError>;
 
-    async fn list_ownerships(&self) -> Result<Vec<SeqV<OwnershipInfo>>>;
+    async fn list_ownerships(&self) -> Result<Vec<SeqV<OwnershipInfo>>, MetaError>;
 
-    async fn list_udf_ownerships(&self) -> Result<Vec<OwnershipInfo>>;
-    async fn list_stage_ownerships(&self) -> Result<Vec<OwnershipInfo>>;
-    async fn list_seq_ownerships(&self) -> Result<Vec<OwnershipInfo>>;
-    async fn list_procedure_ownerships(&self) -> Result<Vec<OwnershipInfo>>;
-    async fn list_connection_ownerships(&self) -> Result<Vec<OwnershipInfo>>;
-    async fn list_warehouse_ownerships(&self) -> Result<Vec<OwnershipInfo>>;
+    async fn list_udf_ownerships(&self) -> Result<Vec<OwnershipInfo>, MetaError>;
+    async fn list_stage_ownerships(&self) -> Result<Vec<OwnershipInfo>, MetaError>;
+    async fn list_seq_ownerships(&self) -> Result<Vec<OwnershipInfo>, MetaError>;
+    async fn list_procedure_ownerships(&self) -> Result<Vec<OwnershipInfo>, MetaError>;
+    async fn list_connection_ownerships(&self) -> Result<Vec<OwnershipInfo>, MetaError>;
+    async fn list_warehouse_ownerships(&self) -> Result<Vec<OwnershipInfo>, MetaError>;
 
     /// General role update.
     ///
-    /// It fetches the role that matches the specified seq number, update it in place, then write it back with the seq it sees.
+    /// It fetches the role, updates it in place, then writes it back with the seq it sees.
     ///
     /// Seq number ensures there is no other write happens between get and set.
-    #[allow(clippy::ptr_arg)]
-    async fn update_role_with<F>(&self, role: &str, seq: MatchSeq, f: F) -> Result<Option<u64>>
-    where F: FnOnce(&mut RoleInfo) + Send;
+    async fn update_role_with<F>(
+        &self,
+        role: &str,
+        f: F,
+    ) -> Result<Result<u64, UnknownError<role_ident::Resource>>, MetaError>
+    where
+        F: FnOnce(&mut RoleInfo) + Send;
 
     /// Only drop role will call transfer.
     ///
     /// If a role is dropped, but the owner object is exists,
     ///
     /// The owner role need to update to account_admin.
-    async fn transfer_ownership_to_admin(&self, role: &str) -> Result<()>;
+    async fn transfer_ownership_to_admin(&self, role: &str) -> Result<(), MetaTxnError>;
 
     /// Grant ownership would transfer ownership of a object from one role to another role
     ///
@@ -83,24 +86,34 @@ pub trait RoleApi: Sync + Send {
     /// 3. kv api upsert new owner object key.
     /// Note: if role/old_role is `account_admin` or `public` no need to revoke/grant ownership privilege
     #[allow(clippy::ptr_arg)]
-    async fn grant_ownership(&self, object: &OwnershipObject, role: &str) -> Result<()>;
+    async fn grant_ownership(
+        &self,
+        object: &OwnershipObject,
+        role: &str,
+    ) -> Result<(), MetaTxnError>;
 
     /// Remember to call this method when you dropped a OwnerObject like table/database/stage/udf.
     /// Revoke ownership used when drop old object, contains two step:
     /// 1. revoke ownership privilege obj to new role,
     /// 2. kv api delete old owner object key.
     ///
     /// Note: if role is `account_admin` or None no need to revoke
-    async fn revoke_ownership(&self, object: &OwnershipObject) -> Result<()>;
+    async fn revoke_ownership(&self, object: &OwnershipObject) -> Result<(), MetaTxnError>;
 
     /// Get the ownership info by object. If it's not granted to any role, return PUBLIC
-    async fn get_ownership(&self, object: &OwnershipObject) -> Result<Option<OwnershipInfo>>;
+    async fn get_ownership(
+        &self,
+        object: &OwnershipObject,
+    ) -> Result<Option<OwnershipInfo>, MetaError>;
 
     /// Get multiple ownership info by objects in batch.
     async fn mget_ownerships(
         &self,
         objects: &[OwnershipObject],
-    ) -> Result<Vec<Option<OwnershipInfo>>>;
+    ) -> Result<Vec<Option<OwnershipInfo>>, MetaError>;
 
-    async fn drop_role(&self, role: String, seq: MatchSeq) -> Result<()>;
+    async fn drop_role(
+        &self,
+        role: &str,
+    ) -> Result<Result<(), UnknownError<role_ident::Resource>>, MetaError>;
 }