Explorar o código

Simplify kvstore validation with optional key parameter

Change validate_kvstore_params to accept Option<&str> for the key parameter,
allowing kv_list operations to share the same validation logic without
duplicating namespace validation code.
Cesar Rodas hai 1 mes
pai
achega
752649a8e8

+ 12 - 10
crates/cdk-common/src/database/kvstore.rs

@@ -36,7 +36,7 @@ pub fn validate_kvstore_string(s: &str) -> Result<(), Error> {
 pub fn validate_kvstore_params(
     primary_namespace: &str,
     secondary_namespace: &str,
-    key: &str,
+    key: Option<&str>,
 ) -> Result<(), Error> {
     // Validate primary namespace
     validate_kvstore_string(primary_namespace)?;
@@ -44,9 +44,6 @@ pub fn validate_kvstore_params(
     // Validate secondary namespace
     validate_kvstore_string(secondary_namespace)?;
 
-    // Validate key
-    validate_kvstore_string(key)?;
-
     // Check empty namespace rules
     if primary_namespace.is_empty() && !secondary_namespace.is_empty() {
         return Err(Error::KVStoreInvalidKey(
@@ -54,12 +51,17 @@ pub fn validate_kvstore_params(
         ));
     }
 
-    // Check for potential collisions between keys and namespaces in the same namespace
-    let namespace_key = format!("{primary_namespace}/{secondary_namespace}");
-    if key == primary_namespace || key == secondary_namespace || key == namespace_key {
-        return Err(Error::KVStoreInvalidKey(format!(
-            "Key '{key}' conflicts with namespace names"
-        )));
+    if let Some(key) = key {
+        // Validate key
+        validate_kvstore_string(key)?;
+
+        // Check for potential collisions between keys and namespaces in the same namespace
+        let namespace_key = format!("{primary_namespace}/{secondary_namespace}");
+        if key == primary_namespace || key == secondary_namespace || key == namespace_key {
+            return Err(Error::KVStoreInvalidKey(format!(
+                "Key '{key}' conflicts with namespace names"
+            )));
+        }
     }
 
     Ok(())

+ 16 - 16
crates/cdk-common/src/database/mint/test/kvstore.rs

@@ -84,16 +84,16 @@ mod tests {
     #[test]
     fn test_validate_kvstore_params_valid() {
         // Test valid parameter combinations
-        assert!(validate_kvstore_params("primary", "secondary", "key").is_ok());
-        assert!(validate_kvstore_params("primary", "", "key").is_ok());
-        assert!(validate_kvstore_params("", "", "key").is_ok());
-        assert!(validate_kvstore_params("p1", "s1", "different_key").is_ok());
+        assert!(validate_kvstore_params("primary", "secondary", Some("key")).is_ok());
+        assert!(validate_kvstore_params("primary", "", Some("key")).is_ok());
+        assert!(validate_kvstore_params("", "", Some("key")).is_ok());
+        assert!(validate_kvstore_params("p1", "s1", Some("different_key")).is_ok());
     }
 
     #[test]
     fn test_validate_kvstore_params_empty_namespace_rules() {
         // Test empty namespace rules: if primary is empty, secondary must be empty too
-        let result = validate_kvstore_params("", "secondary", "key");
+        let result = validate_kvstore_params("", "secondary", Some("key"));
         assert!(result.is_err());
         assert!(result
             .unwrap_err()
@@ -110,7 +110,7 @@ mod tests {
         ];
 
         for (primary, secondary, key) in test_cases {
-            let result = validate_kvstore_params(primary, secondary, key);
+            let result = validate_kvstore_params(primary, secondary, Some(key));
             assert!(
                 result.is_err(),
                 "Expected collision for key '{}' with namespaces '{}'/'{}'",
@@ -123,20 +123,20 @@ mod tests {
         }
 
         // Test that a combined namespace string would be invalid due to the slash character
-        let result = validate_kvstore_params("primary", "secondary", "primary_secondary");
+        let result = validate_kvstore_params("primary", "secondary", Some("primary_secondary"));
         assert!(result.is_ok(), "This should be valid - no actual collision");
     }
 
     #[test]
     fn test_validate_kvstore_params_invalid_strings() {
         // Test invalid characters in any parameter
-        let result = validate_kvstore_params("primary@", "secondary", "key");
+        let result = validate_kvstore_params("primary@", "secondary", Some("key"));
         assert!(result.is_err());
 
-        let result = validate_kvstore_params("primary", "secondary!", "key");
+        let result = validate_kvstore_params("primary", "secondary!", Some("key"));
         assert!(result.is_err());
 
-        let result = validate_kvstore_params("primary", "secondary", "key with space");
+        let result = validate_kvstore_params("primary", "secondary", Some("key with space"));
         assert!(result.is_err());
     }
 
@@ -179,7 +179,7 @@ mod tests {
 
         for (primary, secondary, key) in valid_examples {
             assert!(
-                validate_kvstore_params(primary, secondary, key).is_ok(),
+                validate_kvstore_params(primary, secondary, Some(key)).is_ok(),
                 "Valid example should pass: '{}'/'{}'/'{}'",
                 primary,
                 secondary,
@@ -196,12 +196,12 @@ mod tests {
         // but ensures naming conflicts don't occur between keys and namespaces.
 
         // These should be valid (different namespaces)
-        assert!(validate_kvstore_params("ns1", "sub1", "key1").is_ok());
-        assert!(validate_kvstore_params("ns2", "sub1", "key1").is_ok()); // same key, different primary namespace
-        assert!(validate_kvstore_params("ns1", "sub2", "key1").is_ok()); // same key, different secondary namespace
+        assert!(validate_kvstore_params("ns1", "sub1", Some("key1")).is_ok());
+        assert!(validate_kvstore_params("ns2", "sub1", Some("key1")).is_ok()); // same key, different primary namespace
+        assert!(validate_kvstore_params("ns1", "sub2", Some("key1")).is_ok()); // same key, different secondary namespace
 
         // These should fail (collision within namespace)
-        assert!(validate_kvstore_params("ns1", "sub1", "ns1").is_err()); // key conflicts with primary namespace
-        assert!(validate_kvstore_params("ns1", "sub1", "sub1").is_err()); // key conflicts with secondary namespace
+        assert!(validate_kvstore_params("ns1", "sub1", Some("ns1")).is_err()); // key conflicts with primary namespace
+        assert!(validate_kvstore_params("ns1", "sub1", Some("sub1")).is_err()); // key conflicts with secondary namespace
     }
 }

+ 8 - 25
crates/cdk-redb/src/wallet/mod.rs

@@ -9,8 +9,8 @@ use std::sync::Arc;
 use async_trait::async_trait;
 use cdk_common::common::ProofInfo;
 use cdk_common::database::{
-    validate_kvstore_params, validate_kvstore_string, DbTransactionFinalizer, KVStore,
-    KVStoreDatabase, KVStoreTransaction, WalletDatabase, WalletDatabaseTransaction,
+    validate_kvstore_params, DbTransactionFinalizer, KVStore, KVStoreDatabase, KVStoreTransaction,
+    WalletDatabase, WalletDatabaseTransaction,
 };
 use cdk_common::mint_url::MintUrl;
 use cdk_common::util::unix_time;
@@ -542,7 +542,7 @@ impl KVStoreDatabase for WalletRedbDatabase {
         key: &str,
     ) -> Result<Option<Vec<u8>>, Self::Err> {
         // Validate parameters according to KV store requirements
-        validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+        validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
 
         let read_txn = self.db.begin_read().map_err(Error::from)?;
         let table = read_txn.open_table(KV_STORE_TABLE).map_err(Error::from)?;
@@ -561,16 +561,7 @@ impl KVStoreDatabase for WalletRedbDatabase {
         primary_namespace: &str,
         secondary_namespace: &str,
     ) -> Result<Vec<String>, Self::Err> {
-        // Validate namespace parameters according to KV store requirements
-        validate_kvstore_string(primary_namespace)?;
-        validate_kvstore_string(secondary_namespace)?;
-
-        // Check empty namespace rules
-        if primary_namespace.is_empty() && !secondary_namespace.is_empty() {
-            return Err(database::Error::KVStoreInvalidKey(
-                "If primary_namespace is empty, secondary_namespace must also be empty".to_string(),
-            ));
-        }
+        validate_kvstore_params(primary_namespace, secondary_namespace, None)?;
 
         let read_txn = self.db.begin_read().map_err(Error::from)?;
         let table = read_txn.open_table(KV_STORE_TABLE).map_err(Error::from)?;
@@ -1105,7 +1096,7 @@ impl KVStoreTransaction<database::Error> for RedbWalletTransaction {
         key: &str,
     ) -> Result<Option<Vec<u8>>, database::Error> {
         // Validate parameters according to KV store requirements
-        validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+        validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
 
         let txn = self.txn()?;
         let table = txn.open_table(KV_STORE_TABLE).map_err(Error::from)?;
@@ -1127,7 +1118,7 @@ impl KVStoreTransaction<database::Error> for RedbWalletTransaction {
         value: &[u8],
     ) -> Result<(), database::Error> {
         // Validate parameters according to KV store requirements
-        validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+        validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
 
         let txn = self.txn()?;
         let mut table = txn.open_table(KV_STORE_TABLE).map_err(Error::from)?;
@@ -1147,7 +1138,7 @@ impl KVStoreTransaction<database::Error> for RedbWalletTransaction {
         key: &str,
     ) -> Result<(), database::Error> {
         // Validate parameters according to KV store requirements
-        validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+        validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
 
         let txn = self.txn()?;
         let mut table = txn.open_table(KV_STORE_TABLE).map_err(Error::from)?;
@@ -1166,15 +1157,7 @@ impl KVStoreTransaction<database::Error> for RedbWalletTransaction {
         secondary_namespace: &str,
     ) -> Result<Vec<String>, database::Error> {
         // Validate namespace parameters according to KV store requirements
-        validate_kvstore_string(primary_namespace)?;
-        validate_kvstore_string(secondary_namespace)?;
-
-        // Check empty namespace rules
-        if primary_namespace.is_empty() && !secondary_namespace.is_empty() {
-            return Err(database::Error::KVStoreInvalidKey(
-                "If primary_namespace is empty, secondary_namespace must also be empty".to_string(),
-            ));
-        }
+        validate_kvstore_params(primary_namespace, secondary_namespace, None)?;
 
         let txn = self.txn()?;
         let table = txn.open_table(KV_STORE_TABLE).map_err(Error::from)?;

+ 7 - 23
crates/cdk-sql-common/src/keyvalue.rs

@@ -5,7 +5,7 @@
 
 use std::sync::Arc;
 
-use cdk_common::database::{validate_kvstore_params, validate_kvstore_string, Error};
+use cdk_common::database::{validate_kvstore_params, Error};
 use cdk_common::util::unix_time;
 
 use crate::column_as_string;
@@ -24,7 +24,7 @@ where
     RM: DatabasePool,
 {
     // Validate parameters according to KV store requirements
-    validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+    validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
     Ok(query(
         r#"
         SELECT value
@@ -57,7 +57,7 @@ where
     RM: DatabasePool,
 {
     // Validate parameters according to KV store requirements
-    validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+    validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
 
     let current_time = unix_time();
 
@@ -95,7 +95,7 @@ where
     RM: DatabasePool,
 {
     // Validate parameters according to KV store requirements
-    validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+    validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
     query(
         r#"
         DELETE FROM kv_store
@@ -123,15 +123,7 @@ where
     RM: DatabasePool,
 {
     // Validate namespace parameters according to KV store requirements
-    validate_kvstore_string(primary_namespace)?;
-    validate_kvstore_string(secondary_namespace)?;
-
-    // Check empty namespace rules
-    if primary_namespace.is_empty() && !secondary_namespace.is_empty() {
-        return Err(Error::KVStoreInvalidKey(
-            "If primary_namespace is empty, secondary_namespace must also be empty".to_string(),
-        ));
-    }
+    validate_kvstore_params(primary_namespace, secondary_namespace, None)?;
     query(
         r#"
         SELECT key
@@ -161,7 +153,7 @@ where
     RM: DatabasePool + 'static,
 {
     // Validate parameters according to KV store requirements
-    validate_kvstore_params(primary_namespace, secondary_namespace, key)?;
+    validate_kvstore_params(primary_namespace, secondary_namespace, Some(key))?;
 
     let conn = pool.get().map_err(|e| Error::Database(Box::new(e)))?;
     Ok(query(
@@ -194,15 +186,7 @@ where
     RM: DatabasePool + 'static,
 {
     // Validate namespace parameters according to KV store requirements
-    validate_kvstore_string(primary_namespace)?;
-    validate_kvstore_string(secondary_namespace)?;
-
-    // Check empty namespace rules
-    if primary_namespace.is_empty() && !secondary_namespace.is_empty() {
-        return Err(Error::KVStoreInvalidKey(
-            "If primary_namespace is empty, secondary_namespace must also be empty".to_string(),
-        ));
-    }
+    validate_kvstore_params(primary_namespace, secondary_namespace, None)?;
 
     let conn = pool.get().map_err(|e| Error::Database(Box::new(e)))?;
     query(