From a85c1c4e19d4c93322ed59d2a209a79245d2fe7e Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Sun, 3 Nov 2024 13:45:44 +0200 Subject: [PATCH 1/8] add test --- examples/acl.rs | 5 +++-- src/macros.rs | 24 ++++++++++++++++-------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/examples/acl.rs b/examples/acl.rs index 9bf071fc..caa0a854 100644 --- a/examples/acl.rs +++ b/examples/acl.rs @@ -25,8 +25,9 @@ redis_module! { version: 1, allocator: (redis_module::alloc::RedisAlloc, redis_module::alloc::RedisAlloc), data_types: [], + acl_category: "acl", commands: [ - ["verify_key_access_for_user", verify_key_access_for_user, "", 0, 0, 0, ""], - ["get_current_user", get_current_user, "", 0, 0, 0, ""], + ["verify_key_access_for_user", verify_key_access_for_user, "", 0, 0, 0, "read"], + ["get_current_user", get_current_user, "", 0, 0, 0, "read"], ], } diff --git a/src/macros.rs b/src/macros.rs index 36f553f3..904288e9 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -7,7 +7,8 @@ macro_rules! redis_command { $firstkey:expr, $lastkey:expr, $keystep:expr, - $acl_categories:expr) => {{ + $acl_categories:expr + ) => {{ let name = CString::new($command_name).unwrap(); let flags = CString::new($command_flags).unwrap(); @@ -134,7 +135,7 @@ macro_rules! redis_module { data_types: [ $($data_type:ident),* $(,)* ], - $(acl_category: $acl_category:expr,)* $(,)* + $(acl_category: $module_acl_categories:expr,)? $(init: $init_func:ident,)* $(,)* $(deinit: $deinit_func:ident,)* $(,)* $(info: $info_func:ident,)? @@ -146,7 +147,7 @@ macro_rules! redis_module { $firstkey:expr, $lastkey:expr, $keystep:expr, - $acl_categories:expr + $command_acl_categories:expr ]),* $(,)* ] $(,)* $(event_handlers: [ @@ -270,18 +271,25 @@ macro_rules! redis_module { } )* + let mut module_acl_categories = CString::new("").unwrap(); $( - let category = CString::new($acl_category).unwrap(); + module_acl_categories = CString::new($module_acl_categories).unwrap(); if let Some(RM_AddACLCategory) = raw::RedisModule_AddACLCategory { - if RM_AddACLCategory(ctx, category.as_ptr()) == raw::Status::Err as c_int { - raw::redis_log(ctx, &format!("Error: failed to add ACL category {}", $acl_category)); + if RM_AddACLCategory(ctx, module_acl_categories.as_ptr()) == raw::Status::Err as c_int { + raw::redis_log(ctx, &format!("Error: failed to add ACL category {}", $module_acl_categories)); return raw::Status::Err as c_int; } } - )* + )? + let module_acl_categories = module_acl_categories.to_str().unwrap(); $( - $crate::redis_command!(ctx, $name, $command, $flags, $firstkey, $lastkey, $keystep, $acl_categories); + let command_acl_categories = if module_acl_categories == "" { + $command_acl_categories.to_string() + } else { + format!("{} {}", module_acl_categories, $command_acl_categories) + }; + $crate::redis_command!(ctx, $name, $command, $flags, $firstkey, $lastkey, $keystep, command_acl_categories.as_str()); )* if $crate::commands::register_commands(&context) == raw::Status::Err { From e23465898b0737e64557160b3568d56b0de34df2 Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Sun, 3 Nov 2024 14:05:36 +0200 Subject: [PATCH 2/8] add comment --- src/macros.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/macros.rs b/src/macros.rs index 904288e9..2501f07e 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -135,6 +135,9 @@ macro_rules! redis_module { data_types: [ $($data_type:ident),* $(,)* ], + // eg: `acl_category: "name_of_module_acl_category",` + // This will add the module to the specified (optional) ACL category. + // All commands will inherit this category. $(acl_category: $module_acl_categories:expr,)? $(init: $init_func:ident,)* $(,)* $(deinit: $deinit_func:ident,)* $(,)* From cdb4965da45712c7f16bd9a595c3160de40f3946 Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Sun, 3 Nov 2024 15:25:02 +0200 Subject: [PATCH 3/8] comments from Meir --- examples/acl.rs | 4 +-- src/macros.rs | 85 ++++++++++++++++++++++++++++++++----------------- 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/examples/acl.rs b/examples/acl.rs index caa0a854..4e7cb5b9 100644 --- a/examples/acl.rs +++ b/examples/acl.rs @@ -27,7 +27,7 @@ redis_module! { data_types: [], acl_category: "acl", commands: [ - ["verify_key_access_for_user", verify_key_access_for_user, "", 0, 0, 0, "read"], - ["get_current_user", get_current_user, "", 0, 0, 0, "read"], + ["verify_key_access_for_user", verify_key_access_for_user, "", 0, 0, 0, "read", "acl"], + ["get_current_user", get_current_user, "", 0, 0, 0, "read", "acl"], ], } diff --git a/src/macros.rs b/src/macros.rs index 2501f07e..6eeefcf0 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -7,7 +7,8 @@ macro_rules! redis_command { $firstkey:expr, $lastkey:expr, $keystep:expr, - $acl_categories:expr + $mandatory_acl_categories:expr + $(, $optional_acl_categories:expr)? ) => {{ let name = CString::new($command_name).unwrap(); let flags = CString::new($command_flags).unwrap(); @@ -38,34 +39,65 @@ macro_rules! redis_command { ) } == $crate::raw::Status::Err as c_int { + $crate::raw::redis_log( + $ctx, + &format!("Error: failed to create command {}", $command_name), + ); return $crate::raw::Status::Err as c_int; } - if $acl_categories != "" { - let acl_categories = CString::new($acl_categories).unwrap(); + let command = + unsafe { $crate::raw::RedisModule_GetCommand.unwrap()($ctx, name.as_ptr()) }; + if command.is_null() { + $crate::raw::redis_log( + $ctx, + &format!("Error: failed to get command {}", $command_name), + ); + return $crate::raw::Status::Err as c_int; + } - let command = - unsafe { $crate::raw::RedisModule_GetCommand.unwrap()($ctx, name.as_ptr()) }; - if command.is_null() { - return $crate::raw::Status::Err as c_int; - } + if let Some(RM_SetCommandACLCategories) = $crate::raw::RedisModule_SetCommandACLCategories { + let mut acl_categories = CString::new("").unwrap(); + $( + if $mandatory_acl_categories != "" && $optional_acl_categories != "" { + acl_categories = CString::new(format!("{} {}", $mandatory_acl_categories, $optional_acl_categories)).unwrap(); + } else if $optional_acl_categories != "" { + acl_categories = CString::new($optional_acl_categories).unwrap(); + } + // Warn if optional ACL categories are not set, but don't fail. + if RM_SetCommandACLCategories(command, acl_categories.as_ptr()) == $crate::raw::Status::Err as c_int { + $crate::raw::redis_log( + $ctx, + &format!( + "Warning: failed to set command `{}` ACL categories `{}`", + $command_name, acl_categories.to_str().unwrap() + ), + ); + } + )? + if $mandatory_acl_categories != "" { + acl_categories = CString::new($mandatory_acl_categories).unwrap(); - if let Some(RM_SetCommandACLCategories) = - $crate::raw::RedisModule_SetCommandACLCategories - { + // Fail if mandatory ACL categories are not set. if RM_SetCommandACLCategories(command, acl_categories.as_ptr()) == $crate::raw::Status::Err as c_int { $crate::raw::redis_log( $ctx, &format!( - "Error: failed to set command {} ACL categories {}", - $command_name, $acl_categories + "Error: failed to set command `{}` mandatory ACL categories `{}`", + $command_name, $mandatory_acl_categories ), ); return $crate::raw::Status::Err as c_int; } } + } else if $mandatory_acl_categories != "" { + $crate::raw::redis_log( + $ctx, + "Warning: Redis version does not support ACL categories", + ); + return $crate::raw::Status::Err as c_int; } }}; } @@ -136,9 +168,8 @@ macro_rules! redis_module { $($data_type:ident),* $(,)* ], // eg: `acl_category: "name_of_module_acl_category",` - // This will add the module to the specified (optional) ACL category. - // All commands will inherit this category. - $(acl_category: $module_acl_categories:expr,)? + // This will add the specified (optional) ACL categories. + $(acl_category: $module_acl_categories:expr,)* $(,)* $(init: $init_func:ident,)* $(,)* $(deinit: $deinit_func:ident,)* $(,)* $(info: $info_func:ident,)? @@ -150,7 +181,8 @@ macro_rules! redis_module { $firstkey:expr, $lastkey:expr, $keystep:expr, - $command_acl_categories:expr + $mandatory_command_acl_categories:expr + $(, $optional_command_acl_categories:expr)? ]),* $(,)* ] $(,)* $(event_handlers: [ @@ -274,25 +306,20 @@ macro_rules! redis_module { } )* - let mut module_acl_categories = CString::new("").unwrap(); $( - module_acl_categories = CString::new($module_acl_categories).unwrap(); + let categories = CString::new($module_acl_categories).unwrap(); if let Some(RM_AddACLCategory) = raw::RedisModule_AddACLCategory { - if RM_AddACLCategory(ctx, module_acl_categories.as_ptr()) == raw::Status::Err as c_int { - raw::redis_log(ctx, &format!("Error: failed to add ACL category {}", $module_acl_categories)); + if RM_AddACLCategory(ctx, categories.as_ptr()) == raw::Status::Err as c_int { + raw::redis_log(ctx, &format!("Error: failed to add ACL categories `{}`", $module_acl_categories)); return raw::Status::Err as c_int; } + } else { + raw::redis_log(ctx, "Warning: Redis version does not support adding new ACL categories"); } - )? + )* - let module_acl_categories = module_acl_categories.to_str().unwrap(); $( - let command_acl_categories = if module_acl_categories == "" { - $command_acl_categories.to_string() - } else { - format!("{} {}", module_acl_categories, $command_acl_categories) - }; - $crate::redis_command!(ctx, $name, $command, $flags, $firstkey, $lastkey, $keystep, command_acl_categories.as_str()); + $crate::redis_command!(ctx, $name, $command, $flags, $firstkey, $lastkey, $keystep, $mandatory_command_acl_categories $(, $optional_command_acl_categories)?); )* if $crate::commands::register_commands(&context) == raw::Status::Err { From a2451d14885ed0d4a903f8216c46031d1a351bc8 Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Sun, 3 Nov 2024 16:13:42 +0200 Subject: [PATCH 4/8] add else --- src/macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macros.rs b/src/macros.rs index 6eeefcf0..99ed2297 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -73,7 +73,7 @@ macro_rules! redis_command { $command_name, acl_categories.to_str().unwrap() ), ); - } + } else )? if $mandatory_acl_categories != "" { acl_categories = CString::new($mandatory_acl_categories).unwrap(); From 3388925029df5f9ad030f388a786c8f54e62ce65 Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Sun, 3 Nov 2024 18:20:58 +0200 Subject: [PATCH 5/8] tests --- src/include/redismodule.h | 1 + src/macros.rs | 4 ++-- test.sh | 2 +- tests/integration.rs | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/include/redismodule.h b/src/include/redismodule.h index b65a552b..d0597755 100644 --- a/src/include/redismodule.h +++ b/src/include/redismodule.h @@ -1326,6 +1326,7 @@ static void RedisModule_InitAPI(RedisModuleCtx *ctx) { REDISMODULE_GET_API(CreateSubcommand); REDISMODULE_GET_API(SetCommandInfo); REDISMODULE_GET_API(SetCommandACLCategories); + REDISMODULE_GET_API(AddACLCategory); REDISMODULE_GET_API(SetModuleAttribs); REDISMODULE_GET_API(IsModuleNameBusy); REDISMODULE_GET_API(WrongArity); diff --git a/src/macros.rs b/src/macros.rs index 99ed2297..c8aac1bb 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -95,7 +95,7 @@ macro_rules! redis_command { } else if $mandatory_acl_categories != "" { $crate::raw::redis_log( $ctx, - "Warning: Redis version does not support ACL categories", + "Error: Redis version does not support ACL categories", ); return $crate::raw::Status::Err as c_int; } @@ -307,8 +307,8 @@ macro_rules! redis_module { )* $( - let categories = CString::new($module_acl_categories).unwrap(); if let Some(RM_AddACLCategory) = raw::RedisModule_AddACLCategory { + let categories = CString::new($module_acl_categories).unwrap(); if RM_AddACLCategory(ctx, categories.as_ptr()) == raw::Status::Err as c_int { raw::redis_log(ctx, &format!("Error: failed to add ACL categories `{}`", $module_acl_categories)); return raw::Status::Err as c_int; diff --git a/test.sh b/test.sh index cc391715..89e605c5 100755 --- a/test.sh +++ b/test.sh @@ -1,2 +1,2 @@ #!/usr/bin/env sh -cargo test --all --all-targets --no-default-features +cargo test --all --all-targets --no-default-features --features min-redis-compatibility-version-7-4 diff --git a/tests/integration.rs b/tests/integration.rs index 114da8fc..8492c84b 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -329,6 +329,40 @@ fn test_get_current_user() -> Result<()> { Ok(()) } +#[test] +#[cfg( + feature = "min-redis-compatibility-version-7-4", +)] +fn test_set_acl_categories() -> Result<()> { + let port: u16 = 6490; + let _guards = vec![start_redis_server_with_module("acl", port) + .with_context(|| "failed to start redis server")?]; + let mut con = + get_redis_connection(port).with_context(|| "failed to connect to redis server")?; + + let res: Vec = redis::cmd("ACL").arg("CAT").query(&mut con)?; + assert!(res.contains(&"acl".to_owned())); + + Ok(()) +} + +#[test] +#[cfg( + feature = "min-redis-compatibility-version-8-0", +)] +fn test_set_acl_categories_commands() -> Result<()> { + let port: u16 = 6490; + let _guards = vec![start_redis_server_with_module("acl", port) + .with_context(|| "failed to start redis server")?]; + let mut con = + get_redis_connection(port).with_context(|| "failed to connect to redis server")?; + + let res: Vec = redis::cmd("ACL").arg("CAT").arg("acl").query(&mut con)?; + assert!(res.contains(&"verify_key_access_for_user".to_owned()) && res.contains(&"get_current_user".to_owned())); + + Ok(()) +} + #[test] fn test_verify_acl_on_user() -> Result<()> { let port: u16 = 6491; From a6e42035edfac159a6124d0bc74e5bb919d2d1b0 Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Mon, 4 Nov 2024 11:04:22 +0200 Subject: [PATCH 6/8] formatting --- tests/integration.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/integration.rs b/tests/integration.rs index 8492c84b..09ce522c 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -330,9 +330,7 @@ fn test_get_current_user() -> Result<()> { } #[test] -#[cfg( - feature = "min-redis-compatibility-version-7-4", -)] +#[cfg(feature = "min-redis-compatibility-version-7-4")] fn test_set_acl_categories() -> Result<()> { let port: u16 = 6490; let _guards = vec![start_redis_server_with_module("acl", port) @@ -347,9 +345,7 @@ fn test_set_acl_categories() -> Result<()> { } #[test] -#[cfg( - feature = "min-redis-compatibility-version-8-0", -)] +#[cfg(feature = "min-redis-compatibility-version-8-0")] fn test_set_acl_categories_commands() -> Result<()> { let port: u16 = 6490; let _guards = vec![start_redis_server_with_module("acl", port) @@ -358,7 +354,10 @@ fn test_set_acl_categories_commands() -> Result<()> { get_redis_connection(port).with_context(|| "failed to connect to redis server")?; let res: Vec = redis::cmd("ACL").arg("CAT").arg("acl").query(&mut con)?; - assert!(res.contains(&"verify_key_access_for_user".to_owned()) && res.contains(&"get_current_user".to_owned())); + assert!( + res.contains(&"verify_key_access_for_user".to_owned()) + && res.contains(&"get_current_user".to_owned()) + ); Ok(()) } From 0f33019fd2f408e9bc0862499adf625176f09f5c Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Mon, 4 Nov 2024 16:08:13 +0200 Subject: [PATCH 7/8] implement `enum AclCategory` --- examples/acl.rs | 10 ++--- src/context/mod.rs | 107 +++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/macros.rs | 51 ++++++++++++--------- 4 files changed, 144 insertions(+), 25 deletions(-) diff --git a/examples/acl.rs b/examples/acl.rs index 4e7cb5b9..fa648ca1 100644 --- a/examples/acl.rs +++ b/examples/acl.rs @@ -1,6 +1,6 @@ use redis_module::{ - redis_module, AclPermissions, Context, NextArg, RedisError, RedisResult, RedisString, - RedisValue, + redis_module, AclCategory, AclPermissions, Context, NextArg, RedisError, RedisResult, + RedisString, RedisValue, }; fn verify_key_access_for_user(ctx: &Context, args: Vec) -> RedisResult { @@ -25,9 +25,9 @@ redis_module! { version: 1, allocator: (redis_module::alloc::RedisAlloc, redis_module::alloc::RedisAlloc), data_types: [], - acl_category: "acl", + acl_categories: [AclCategory::from("acl"), ], commands: [ - ["verify_key_access_for_user", verify_key_access_for_user, "", 0, 0, 0, "read", "acl"], - ["get_current_user", get_current_user, "", 0, 0, 0, "read", "acl"], + ["verify_key_access_for_user", verify_key_access_for_user, "", 0, 0, 0, AclCategory::Read, AclCategory::from("acl")], + ["get_current_user", get_current_user, "", 0, 0, 0, vec![AclCategory::Read, AclCategory::Fast], AclCategory::from("acl")], ], } diff --git a/src/context/mod.rs b/src/context/mod.rs index 5d0432a8..d739d531 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -926,6 +926,113 @@ bitflags! { } } +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub enum AclCategory { + #[default] + None, + Keyspace, + Read, + Write, + Set, + SortedSet, + List, + Hash, + String, + Bitmap, + HyperLogLog, + Geo, + Stream, + PubSub, + Admin, + Fast, + Slow, + Blocking, + Dangerous, + Connection, + Transaction, + Scripting, + Single(String), + Multi(Vec), +} + +impl From> for AclCategory { + fn from(value: Vec) -> Self { + AclCategory::Multi(value) + } +} + +impl From<&str> for AclCategory { + fn from(value: &str) -> Self { + match value { + "" => AclCategory::None, + "keyspace" => AclCategory::Keyspace, + "read" => AclCategory::Read, + "write" => AclCategory::Write, + "set" => AclCategory::Set, + "sortedset" => AclCategory::SortedSet, + "list" => AclCategory::List, + "hash" => AclCategory::Hash, + "string" => AclCategory::String, + "bitmap" => AclCategory::Bitmap, + "hyperloglog" => AclCategory::HyperLogLog, + "geo" => AclCategory::Geo, + "stream" => AclCategory::Stream, + "pubsub" => AclCategory::PubSub, + "admin" => AclCategory::Admin, + "fast" => AclCategory::Fast, + "slow" => AclCategory::Slow, + "blocking" => AclCategory::Blocking, + "dangerous" => AclCategory::Dangerous, + "connection" => AclCategory::Connection, + "transaction" => AclCategory::Transaction, + "scripting" => AclCategory::Scripting, + _ if !value.contains(" ") => AclCategory::Single(value.to_string()), + _ => AclCategory::Multi(value.split_whitespace().map(AclCategory::from).collect()), + } + } +} + +impl From for String { + fn from(value: AclCategory) -> Self { + match value { + AclCategory::None => "".to_string(), + AclCategory::Keyspace => "keyspace".to_string(), + AclCategory::Read => "read".to_string(), + AclCategory::Write => "write".to_string(), + AclCategory::Set => "set".to_string(), + AclCategory::SortedSet => "sortedset".to_string(), + AclCategory::List => "list".to_string(), + AclCategory::Hash => "hash".to_string(), + AclCategory::String => "string".to_string(), + AclCategory::Bitmap => "bitmap".to_string(), + AclCategory::HyperLogLog => "hyperloglog".to_string(), + AclCategory::Geo => "geo".to_string(), + AclCategory::Stream => "stream".to_string(), + AclCategory::PubSub => "pubsub".to_string(), + AclCategory::Admin => "admin".to_string(), + AclCategory::Fast => "fast".to_string(), + AclCategory::Slow => "slow".to_string(), + AclCategory::Blocking => "blocking".to_string(), + AclCategory::Dangerous => "dangerous".to_string(), + AclCategory::Connection => "connection".to_string(), + AclCategory::Transaction => "transaction".to_string(), + AclCategory::Scripting => "scripting".to_string(), + AclCategory::Single(s) => s, + AclCategory::Multi(v) => v + .into_iter() + .map(String::from) + .collect::>() + .join(" "), + } + } +} + +impl std::fmt::Display for AclCategory { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", String::from(self.clone())) + } +} + /// The values allowed in the "info" sections and dictionaries. #[derive(Debug, Clone)] pub enum InfoContextBuilderFieldBottomLevelValue { diff --git a/src/lib.rs b/src/lib.rs index 9a11b0be..b4195b78 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,6 +33,7 @@ pub use crate::context::commands; pub use crate::context::defrag; pub use crate::context::keys_cursor::KeysCursor; pub use crate::context::server_events; +pub use crate::context::AclCategory; pub use crate::context::AclPermissions; #[cfg(any( feature = "min-redis-compatibility-version-7-4", diff --git a/src/macros.rs b/src/macros.rs index c8aac1bb..ef86d351 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -10,6 +10,8 @@ macro_rules! redis_command { $mandatory_acl_categories:expr $(, $optional_acl_categories:expr)? ) => {{ + use redis_module::AclCategory; + let name = CString::new($command_name).unwrap(); let flags = CString::new($command_flags).unwrap(); @@ -56,13 +58,15 @@ macro_rules! redis_command { return $crate::raw::Status::Err as c_int; } + let mandatory_acl_categories = AclCategory::from($mandatory_acl_categories); if let Some(RM_SetCommandACLCategories) = $crate::raw::RedisModule_SetCommandACLCategories { - let mut acl_categories = CString::new("").unwrap(); + let mut acl_categories = CString::default(); $( - if $mandatory_acl_categories != "" && $optional_acl_categories != "" { - acl_categories = CString::new(format!("{} {}", $mandatory_acl_categories, $optional_acl_categories)).unwrap(); - } else if $optional_acl_categories != "" { - acl_categories = CString::new($optional_acl_categories).unwrap(); + let optional_acl_categories = AclCategory::from($optional_acl_categories); + if mandatory_acl_categories != AclCategory::None && optional_acl_categories != AclCategory::None { + acl_categories = CString::new(format!("{} {}", mandatory_acl_categories, optional_acl_categories)).unwrap(); + } else if optional_acl_categories != AclCategory::None { + acl_categories = CString::new(format!("{}", $optional_acl_categories)).unwrap(); } // Warn if optional ACL categories are not set, but don't fail. if RM_SetCommandACLCategories(command, acl_categories.as_ptr()) == $crate::raw::Status::Err as c_int { @@ -75,8 +79,8 @@ macro_rules! redis_command { ); } else )? - if $mandatory_acl_categories != "" { - acl_categories = CString::new($mandatory_acl_categories).unwrap(); + if mandatory_acl_categories != AclCategory::None { + acl_categories = CString::new(format!("{}", mandatory_acl_categories)).unwrap(); // Fail if mandatory ACL categories are not set. if RM_SetCommandACLCategories(command, acl_categories.as_ptr()) @@ -86,13 +90,13 @@ macro_rules! redis_command { $ctx, &format!( "Error: failed to set command `{}` mandatory ACL categories `{}`", - $command_name, $mandatory_acl_categories + $command_name, mandatory_acl_categories ), ); return $crate::raw::Status::Err as c_int; } } - } else if $mandatory_acl_categories != "" { + } else if mandatory_acl_categories != AclCategory::None { $crate::raw::redis_log( $ctx, "Error: Redis version does not support ACL categories", @@ -167,9 +171,11 @@ macro_rules! redis_module { data_types: [ $($data_type:ident),* $(,)* ], - // eg: `acl_category: "name_of_module_acl_category",` + // eg: `acl_category: [ "name_of_module_acl_category", ],` // This will add the specified (optional) ACL categories. - $(acl_category: $module_acl_categories:expr,)* $(,)* + $(acl_categories: [ + $($module_acl_category:expr,)* + ],)? $(init: $init_func:ident,)* $(,)* $(deinit: $deinit_func:ident,)* $(,)* $(info: $info_func:ident,)? @@ -307,16 +313,21 @@ macro_rules! redis_module { )* $( - if let Some(RM_AddACLCategory) = raw::RedisModule_AddACLCategory { - let categories = CString::new($module_acl_categories).unwrap(); - if RM_AddACLCategory(ctx, categories.as_ptr()) == raw::Status::Err as c_int { - raw::redis_log(ctx, &format!("Error: failed to add ACL categories `{}`", $module_acl_categories)); - return raw::Status::Err as c_int; + $( + if let Some(RM_AddACLCategory) = raw::RedisModule_AddACLCategory { + let module_acl_category = AclCategory::from($module_acl_category); + if module_acl_category != AclCategory::None { + let category = CString::new(format!("{}", $module_acl_category)).unwrap(); + if RM_AddACLCategory(ctx, category.as_ptr()) == raw::Status::Err as c_int { + raw::redis_log(ctx, &format!("Error: failed to add ACL category `{}`", $module_acl_category)); + return raw::Status::Err as c_int; + } + } + } else { + raw::redis_log(ctx, "Warning: Redis version does not support adding new ACL categories"); } - } else { - raw::redis_log(ctx, "Warning: Redis version does not support adding new ACL categories"); - } - )* + )* + )? $( $crate::redis_command!(ctx, $name, $command, $flags, $firstkey, $lastkey, $keystep, $mandatory_command_acl_categories $(, $optional_command_acl_categories)?); From fc51c7e5312c1572b95f68c93e9288e88e21846c Mon Sep 17 00:00:00 2001 From: Ephraim Feldblum Date: Mon, 4 Nov 2024 16:13:42 +0200 Subject: [PATCH 8/8] using `TestConnection` API --- tests/integration.rs | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tests/integration.rs b/tests/integration.rs index e94a1bd5..6a66c451 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -278,11 +278,7 @@ fn test_get_current_user() -> Result<()> { #[test] #[cfg(feature = "min-redis-compatibility-version-7-4")] fn test_set_acl_categories() -> Result<()> { - let port: u16 = 6490; - let _guards = vec![start_redis_server_with_module("acl", port) - .with_context(|| "failed to start redis server")?]; - let mut con = - get_redis_connection(port).with_context(|| "failed to connect to redis server")?; + let mut con = TestConnection::new("acl"); let res: Vec = redis::cmd("ACL").arg("CAT").query(&mut con)?; assert!(res.contains(&"acl".to_owned())); @@ -293,11 +289,7 @@ fn test_set_acl_categories() -> Result<()> { #[test] #[cfg(feature = "min-redis-compatibility-version-8-0")] fn test_set_acl_categories_commands() -> Result<()> { - let port: u16 = 6490; - let _guards = vec![start_redis_server_with_module("acl", port) - .with_context(|| "failed to start redis server")?]; - let mut con = - get_redis_connection(port).with_context(|| "failed to connect to redis server")?; + let mut con = TestConnection::new("acl"); let res: Vec = redis::cmd("ACL").arg("CAT").arg("acl").query(&mut con)?; assert!(