refactor: Extract kvapi::KeyCodec trait from kvapi::Key (#15262)

The `kvapi::Key` trait encompasses various key behaviors, including prefix, parent, and value type. The key codec functionality, responsible for encoding and decoding components of a key, operates independently of these behaviors. This is evident in scenarios like a database identifier `(tenant, db_name)`, where `db_name` requires its own distinct codec logic independent of the overarching key structure. This commit introduces the `kvapi::KeyCodec` trait, which abstracts the codec functionality with two primary methods: `encode_key()` and `decode_key()`. This separation enhances modularity and aligns with single responsibility principles, allowing for more targeted implementations and testing of key encoding and decoding behaviors.
databendlabs · Apr 19, 2024 · d440030 · d440030
1 parent 701b1d9
commit d440030
Show file tree

Hide file tree

Showing 18 changed files with 305 additions and 230 deletions.
diff --git a/src/meta/app/src/background/background_job.rs b/src/meta/app/src/background/background_job.rs
@@ -366,15 +366,7 @@ mod kvapi_key_impl {
     use crate::background::background_job::BackgroundJobId;
     use crate::background::BackgroundJobInfo;
 
-    impl kvapi::Key for BackgroundJobId {
-        const PREFIX: &'static str = "__fd_background_job_by_id";
-
-        type ValueType = BackgroundJobInfo;
-
-        fn parent(&self) -> Option<String> {
-            None
-        }
-
+    impl kvapi::KeyCodec for BackgroundJobId {
         fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
             b.push_u64(self.id)
         }
@@ -385,6 +377,16 @@ mod kvapi_key_impl {
         }
     }
 
+    impl kvapi::Key for BackgroundJobId {
+        const PREFIX: &'static str = "__fd_background_job_by_id";
+
+        type ValueType = BackgroundJobInfo;
+
+        fn parent(&self) -> Option<String> {
+            None
+        }
+    }
+
     impl kvapi::Value for BackgroundJobInfo {
         fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
             []

diff --git a/src/meta/app/src/data_mask/mod.rs b/src/meta/app/src/data_mask/mod.rs
@@ -110,6 +110,18 @@ mod kvapi_key_impl {
     use super::DatamaskId;
     use crate::data_mask::DatamaskMeta;
 
+    impl kvapi::KeyCodec for DatamaskId {
+        fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
+            b.push_u64(self.id)
+        }
+
+        fn decode_key(parser: &mut KeyParser) -> Result<Self, KeyError>
+        where Self: Sized {
+            let id = parser.next_u64()?;
+            Ok(Self { id })
+        }
+    }
+
     /// "__fd_datamask_by_id/<id>"
     impl kvapi::Key for DatamaskId {
         const PREFIX: &'static str = "__fd_datamask_by_id";
@@ -119,15 +131,6 @@ mod kvapi_key_impl {
         fn parent(&self) -> Option<String> {
             None
         }
-
-        fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
-            b.push_u64(self.id)
-        }
-
-        fn decode_key(parser: &mut KeyParser) -> Result<Self, KeyError> {
-            let id = parser.next_u64()?;
-            Ok(Self { id })
-        }
     }
 
     impl kvapi::Value for DatamaskMeta {

diff --git a/src/meta/app/src/id_generator.rs b/src/meta/app/src/id_generator.rs
@@ -99,15 +99,7 @@ impl IdGenerator {
     }
 }
 
-impl kvapi::Key for IdGenerator {
-    const PREFIX: &'static str = "__fd_id_gen";
-
-    type ValueType = Infallible;
-
-    fn parent(&self) -> Option<String> {
-        None
-    }
-
+impl kvapi::KeyCodec for IdGenerator {
     fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
         b.push_raw(&self.resource)
     }
@@ -121,6 +113,16 @@ impl kvapi::Key for IdGenerator {
     }
 }
 
+impl kvapi::Key for IdGenerator {
+    const PREFIX: &'static str = "__fd_id_gen";
+
+    type ValueType = Infallible;
+
+    fn parent(&self) -> Option<String> {
+        None
+    }
+}
+
 #[cfg(test)]
 mod t {
     use databend_common_meta_kvapi::kvapi::Key;

diff --git a/src/meta/app/src/principal/user_grant.rs b/src/meta/app/src/principal/user_grant.rs
@@ -468,14 +468,7 @@ mod kvapi_key_impl {
     use crate::tenant::Tenant;
     use crate::KeyWithTenant;
 
-    impl kvapi::Key for TenantOwnershipObject {
-        const PREFIX: &'static str = "__fd_object_owners";
-        type ValueType = OwnershipInfo;
-
-        fn parent(&self) -> Option<String> {
-            Some(self.tenant.to_string_key())
-        }
-
+    impl kvapi::KeyCodec for TenantOwnershipObject {
         fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
             let b = b.push_str(self.tenant_name());
             self.object.build_key(b)
@@ -492,6 +485,15 @@ mod kvapi_key_impl {
         }
     }
 
+    impl kvapi::Key for TenantOwnershipObject {
+        const PREFIX: &'static str = "__fd_object_owners";
+        type ValueType = OwnershipInfo;
+
+        fn parent(&self) -> Option<String> {
+            Some(self.tenant.to_string_key())
+        }
+    }
+
     impl kvapi::Value for OwnershipInfo {
         fn dependency_keys(&self) -> impl IntoIterator<Item = String> {
             []

diff --git a/src/meta/app/src/principal/user_identity.rs b/src/meta/app/src/principal/user_identity.rs
@@ -96,14 +96,7 @@ mod kvapi_key_impl {
     use crate::tenant::Tenant;
     use crate::KeyWithTenant;
 
-    impl kvapi::Key for TenantUserIdent {
-        const PREFIX: &'static str = "__fd_users";
-        type ValueType = UserInfo;
-
-        fn parent(&self) -> Option<String> {
-            Some(self.tenant.to_string_key())
-        }
-
+    impl kvapi::KeyCodec for TenantUserIdent {
         fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
             b.push_str(self.tenant_name())
                 .push_str(&self.user.to_string())
@@ -122,6 +115,15 @@ mod kvapi_key_impl {
         }
     }
 
+    impl kvapi::Key for TenantUserIdent {
+        const PREFIX: &'static str = "__fd_users";
+        type ValueType = UserInfo;
+
+        fn parent(&self) -> Option<String> {
+            Some(self.tenant.to_string_key())
+        }
+    }
+
     impl KeyWithTenant for TenantUserIdent {
         fn tenant(&self) -> &Tenant {
             &self.tenant

diff --git a/src/meta/app/src/principal/user_stage_file_ident.rs b/src/meta/app/src/principal/user_stage_file_ident.rs
@@ -42,14 +42,7 @@ mod kvapi_key_impl {
     use crate::tenant::Tenant;
     use crate::KeyWithTenant;
 
-    impl kvapi::Key for StageFileIdent {
-        const PREFIX: &'static str = "__fd_stage_files";
-        type ValueType = StageFile;
-
-        fn parent(&self) -> Option<String> {
-            Some(self.stage.to_string_key())
-        }
-
+    impl kvapi::KeyCodec for StageFileIdent {
         fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
             b.push_str(self.stage.tenant_name())
                 .push_str(self.stage.name())
@@ -63,6 +56,15 @@ mod kvapi_key_impl {
         }
     }
 
+    impl kvapi::Key for StageFileIdent {
+        const PREFIX: &'static str = "__fd_stage_files";
+        type ValueType = StageFile;
+
+        fn parent(&self) -> Option<String> {
+            Some(self.stage.to_string_key())
+        }
+    }
+
     impl KeyWithTenant for StageFileIdent {
         fn tenant(&self) -> &Tenant {
             self.stage.tenant()

diff --git a/src/meta/app/src/schema/catalog.rs b/src/meta/app/src/schema/catalog.rs
@@ -283,6 +283,18 @@ mod kvapi_key_impl {
     use crate::schema::CatalogMeta;
     use crate::schema::CatalogNameIdent;
 
+    impl kvapi::KeyCodec for CatalogId {
+        fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
+            b.push_u64(self.catalog_id)
+        }
+
+        fn decode_key(parser: &mut KeyParser) -> Result<Self, KeyError> {
+            let catalog_id = parser.next_u64()?;
+
+            Ok(Self { catalog_id })
+        }
+    }
+
     /// "__fd_catalog_by_id/<catalog_id>"
     impl kvapi::Key for CatalogId {
         const PREFIX: &'static str = "__fd_catalog_by_id";
@@ -292,7 +304,9 @@ mod kvapi_key_impl {
         fn parent(&self) -> Option<String> {
             None
         }
+    }
 
+    impl kvapi::KeyCodec for CatalogIdToName {
         fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
             b.push_u64(self.catalog_id)
         }
@@ -313,16 +327,6 @@ mod kvapi_key_impl {
         fn parent(&self) -> Option<String> {
             Some(CatalogId::new(self.catalog_id).to_string_key())
         }
-
-        fn encode_key(&self, b: KeyBuilder) -> KeyBuilder {
-            b.push_u64(self.catalog_id)
-        }
-
-        fn decode_key(parser: &mut KeyParser) -> Result<Self, KeyError> {
-            let catalog_id = parser.next_u64()?;
-
-            Ok(Self { catalog_id })
-        }
     }
 
     impl kvapi::Value for CatalogMeta {

diff --git a/src/meta/app/src/schema/database.rs b/src/meta/app/src/schema/database.rs
@@ -364,6 +364,17 @@ mod kvapi_key_impl {
     use crate::schema::DbIdListKey;
     use crate::tenant::Tenant;
 
+    impl kvapi::KeyCodec for DatabaseId {
+        fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
+            b.push_u64(self.db_id)
+        }
+
+        fn decode_key(parser: &mut kvapi::KeyParser) -> Result<Self, kvapi::KeyError> {
+            let db_id = parser.next_u64()?;
+            Ok(Self { db_id })
+        }
+    }
+
     /// "__fd_database_by_id/<db_id>"
     impl kvapi::Key for DatabaseId {
         const PREFIX: &'static str = "__fd_database_by_id";
@@ -373,7 +384,9 @@ mod kvapi_key_impl {
         fn parent(&self) -> Option<String> {
             None
         }
+    }
 
+    impl kvapi::KeyCodec for DatabaseIdToName {
         fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
             b.push_u64(self.db_id)
         }
@@ -393,14 +406,21 @@ mod kvapi_key_impl {
         fn parent(&self) -> Option<String> {
             Some(DatabaseId::new(self.db_id).to_string_key())
         }
+    }
 
+    impl kvapi::KeyCodec for DbIdListKey {
         fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
-            b.push_u64(self.db_id)
+            b.push_str(self.tenant.tenant_name())
+                .push_str(&self.db_name)
         }
 
         fn decode_key(parser: &mut kvapi::KeyParser) -> Result<Self, kvapi::KeyError> {
-            let db_id = parser.next_u64()?;
-            Ok(Self { db_id })
+            let tenant = parser.next_nonempty()?;
+            let db_name = parser.next_str()?;
+
+            let tenant = Tenant::new_nonempty(tenant);
+
+            Ok(Self { tenant, db_name })
         }
     }
 
@@ -413,20 +433,6 @@ mod kvapi_key_impl {
         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.tenant_name())
-                .push_str(&self.db_name)
-        }
-
-        fn decode_key(parser: &mut kvapi::KeyParser) -> Result<Self, kvapi::KeyError> {
-            let tenant = parser.next_nonempty()?;
-            let db_name = parser.next_str()?;
-
-            let tenant = Tenant::new_nonempty(tenant);
-
-            Ok(Self { tenant, db_name })
-        }
     }
 
     impl kvapi::Value for DatabaseMeta {

diff --git a/src/meta/app/src/schema/index.rs b/src/meta/app/src/schema/index.rs
@@ -240,6 +240,18 @@ mod kvapi_key_impl {
     use crate::schema::IndexMeta;
     use crate::schema::IndexNameIdentRaw;
 
+    impl kvapi::KeyCodec for IndexId {
+        fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
+            b.push_u64(self.index_id)
+        }
+
+        fn decode_key(parser: &mut kvapi::KeyParser) -> Result<Self, kvapi::KeyError> {
+            let index_id = parser.next_u64()?;
+
+            Ok(Self { index_id })
+        }
+    }
+
     /// "<prefix>/<index_id>"
     impl kvapi::Key for IndexId {
         const PREFIX: &'static str = "__fd_index_by_id";
@@ -249,7 +261,9 @@ mod kvapi_key_impl {
         fn parent(&self) -> Option<String> {
             None
         }
+    }
 
+    impl kvapi::KeyCodec for IndexIdToName {
         fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
             b.push_u64(self.index_id)
         }
@@ -270,16 +284,6 @@ mod kvapi_key_impl {
         fn parent(&self) -> Option<String> {
             Some(IndexId::new(self.index_id).to_string_key())
         }
-
-        fn encode_key(&self, b: kvapi::KeyBuilder) -> kvapi::KeyBuilder {
-            b.push_u64(self.index_id)
-        }
-
-        fn decode_key(parser: &mut kvapi::KeyParser) -> Result<Self, kvapi::KeyError> {
-            let index_id = parser.next_u64()?;
-
-            Ok(Self { index_id })
-        }
     }
 
     impl kvapi::Value for IndexMeta {