From 13101c079e6b472fca8c870b036412e84da13544 Mon Sep 17 00:00:00 2001 From: meir Date: Sun, 7 May 2023 14:48:35 +0300 Subject: [PATCH 01/10] The PR introduce a new proc macro for command registrations. The PR introduce a new proc macro for command registrations with the new command registration API that was introduced on Redis 7. The new proc macro are called `redis_command`, The following command will register the `test_command` function as a Redis command called `foo`: ```rust #[redis_command( { name: "foo", arity: 3, key_spec: [ { notes: "some notes", flags: ["RW", "ACCESS"], begin_search: Keyword(("foo", 1)), find_keys: Range((1, 2, 3)), } ] } )] fn test_command(_ctx: &Context, _args: Vec) -> RedisResult { Ok(RedisValue::SimpleStringStatic("OK")) } ``` The supported properties are: * name - The command name, * flags (optional) - Command flags such as `readonly`, for the full list please refer to https://redis.io/docs/reference/modules/modules-api-ref/#redismodule_createcommand * summary (optional) - Command summary * complexity (optional) - Command compexity * since (optional) - At which module version the command was first introduce * tips (optional) - Command tips for proxy, for more information please refer to https://redis.io/topics/command-tips * arity - Number of arguments, including the command name itself. A positive number specifies an exact number of arguments and a negative number specifies a minimum number of arguments. * key_spec - A list of specs representing how to find the keys that the command might touch. the following options are available: * notes (optional) - Some note about the key spec. * flags - List of flags reprenting how the keys are accessed, the following options are available: * RO - Read-Only. Reads the value of the key, but doesn't necessarily return it. * RW - Read-Write. Modifies the data stored in the value of the key or its metadata. * OW - Overwrite. Overwrites the data stored in the value of the key. * RM - Deletes the key. * ACCESS - Returns, copies or uses the user data from the value of the key. * UPDATE - Updates data to the value, new value may depend on the old value. * INSERT - Adds data to the value with no chance of modification or deletion of existing data. * DELETE - Explicitly deletes some content from the value of the key. * NOT_KEY - The key is not actually a key, but should be routed in cluster mode as if it was a key. * INCOMPLETE - The keyspec might not point out all the keys it should cover. * VARIABLE_FLAGS - Some keys might have different flags depending on arguments. * begin_search - Represents how Redis should start looking for keys.There are 2 possible options: * Index - start looking for keys from a given position. * Keyword - Search for a specific keyward and start looking for keys from this keyword * FindKeys - After Redis finds the location from where it needs to start looking for keys, Redis will start finding keys base on the information in this struct. There are 2 possible options: * Range - A tuple represent a range of `(last_key, steps, limit)`. * last_key - Index of the last key relative to the result of the begin search step. Can be negative, in which case it's not relative. -1 indicates the last argument, -2 one before the last and so on. * steps - How many arguments should we skip after finding a key, in order to find the next one. * limit - If `lastkey` is -1, we use `limit` to stop the search by a factor. 0 and 1 mean no limit. 2 means 1/2 of the remaining args, 3 means 1/3, and so on. * Keynum - A tuple of 3 elements `(keynumidx, firstkey, keystep)`. * keynumidx - Index of the argument containing the number of keys to come, relative to the result of the begin search step. * firstkey - Index of the fist key relative to the result of thebegin search step. (Usually it's just after `keynumidx`, inwhich case it should be set to `keynumidx + 1`.) * keystep - How many arguments should we skip after finding a key, in order to find the next one? **Notice**, by default Redis does not validate the command spec. User should validate the command keys on the module command code. The command spec is used for validation on cluster so Redis can raise a cross slot error when needed. Ideas for future extension: * The proc macro can analyze the function inputs and generate a code for parsing the command arguments so that the command function will not have to deal with it. --- Cargo.toml | 4 + examples/proc_macro_commands.rs | 67 +++ .../src/api_versions.rs | 3 + redismodule-rs-macros/Cargo.toml | 3 + redismodule-rs-macros/src/lib.rs | 80 ++++ redismodule-rs-macros/src/redis_command.rs | 178 ++++++++ src/context/commnads.rs | 409 ++++++++++++++++++ src/context/mod.rs | 1 + src/lib.rs | 1 + src/macros.rs | 4 + tests/integration.rs | 39 ++ 11 files changed, 789 insertions(+) create mode 100644 examples/proc_macro_commands.rs create mode 100644 redismodule-rs-macros/src/redis_command.rs create mode 100644 src/context/commnads.rs diff --git a/Cargo.toml b/Cargo.toml index ac8e4d0f..1825e6df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,10 @@ crate-type = ["cdylib"] name = "configuration" crate-type = ["cdylib"] +[[example]] +name = "proc_macro_commands" +crate-type = ["cdylib"] + [[example]] name = "acl" crate-type = ["cdylib"] diff --git a/examples/proc_macro_commands.rs b/examples/proc_macro_commands.rs new file mode 100644 index 00000000..73cb50a2 --- /dev/null +++ b/examples/proc_macro_commands.rs @@ -0,0 +1,67 @@ +use redis_module::{redis_module, Context, RedisResult, RedisString, RedisValue}; +use redis_module_macros::redis_command; + +#[redis_command( + { + name: "classic_keys", + flags: "readonly", + arity: -2, + key_spec: [ + { + notes: "test command that define all the arguments at even possition as keys", + flags: ["RO", "ACCESS"], + begin_search: Index(1), + find_keys: Range((-1, 2, 0)), + } + ] + } +)] +fn classic_keys(_ctx: &Context, _args: Vec) -> RedisResult { + Ok(RedisValue::SimpleStringStatic("OK")) +} + +#[redis_command( + { + name: "keyword_keys", + flags: "readonly", + arity: -2, + key_spec: [ + { + notes: "test command that define all the arguments at even possition as keys", + flags: ["RO", "ACCESS"], + begin_search: Keyword(("foo", 1)), + find_keys: Range((-1, 2, 0)), + } + ] + } +)] +fn keyword_keys(_ctx: &Context, _args: Vec) -> RedisResult { + Ok(RedisValue::SimpleStringStatic("OK")) +} + +#[redis_command( + { + name: "num_keys", + flags: "readonly no-mandatory-keys", + arity: -2, + key_spec: [ + { + notes: "test command that define all the arguments at even possition as keys", + flags: ["RO", "ACCESS"], + begin_search: Index(1), + find_keys: Keynum((0, 1, 1)), + } + ] + } +)] +fn num_keys(_ctx: &Context, _args: Vec) -> RedisResult { + Ok(RedisValue::SimpleStringStatic("OK")) +} + +redis_module! { + name: "server_events", + version: 1, + allocator: (redis_module::alloc::RedisAlloc, redis_module::alloc::RedisAlloc), + data_types: [], + commands: [], +} diff --git a/redismodule-rs-macros-internals/src/api_versions.rs b/redismodule-rs-macros-internals/src/api_versions.rs index 9f02059d..0faf3f55 100644 --- a/redismodule-rs-macros-internals/src/api_versions.rs +++ b/redismodule-rs-macros-internals/src/api_versions.rs @@ -19,6 +19,9 @@ lazy_static::lazy_static! { ("RedisModule_BlockClientSetPrivateData".to_string(), 70200), ("RedisModule_BlockClientOnAuth".to_string(), 70200), ("RedisModule_ACLAddLogEntryByUserName".to_string(), 70200), + ("RedisModule_GetCommand".to_string(), 70000), + ("RedisModule_SetCommandInfo".to_string(), 70000), + ]); pub(crate) static ref API_OLDEST_VERSION: usize = 60000; diff --git a/redismodule-rs-macros/Cargo.toml b/redismodule-rs-macros/Cargo.toml index 7f9354e9..08eb9293 100644 --- a/redismodule-rs-macros/Cargo.toml +++ b/redismodule-rs-macros/Cargo.toml @@ -14,6 +14,9 @@ categories = ["database", "api-bindings"] [dependencies] syn = { version="1.0", features = ["full"]} quote = "1.0" +proc-macro2 = "1" +serde = { version = "1", features = ["derive"] } +serde_syn = "0.1.0" [lib] name = "redis_module_macros" diff --git a/redismodule-rs-macros/src/lib.rs b/redismodule-rs-macros/src/lib.rs index a0633031..e15181b5 100644 --- a/redismodule-rs-macros/src/lib.rs +++ b/redismodule-rs-macros/src/lib.rs @@ -2,6 +2,86 @@ use proc_macro::TokenStream; use quote::quote; use syn::ItemFn; +mod redis_command; + +/// This proc macro allow to specify that the follow function is a Redis command. +/// The macro accept the following arguments that discribe the command properties: +/// * name - The command name, +/// * flags (optional) - Command flags such as `readonly`, for the full list please refer to https://redis.io/docs/reference/modules/modules-api-ref/#redismodule_createcommand +/// * summary (optional) - Command summary +/// * complexity (optional) - Command compexity +/// * since (optional) - At which module version the command was first introduce +/// * tips (optional) - Command tips for proxy, for more information please refer to https://redis.io/topics/command-tips +/// * arity - Number of arguments, including the command name itself. A positive number specifies an exact number of arguments and a negative number +/// specifies a minimum number of arguments. +/// * key_spec - A list of specs representing how to find the keys that the command might touch. the following options are available: +/// * notes (optional) - Some note about the key spec. +/// * flags - List of flags reprenting how the keys are accessed, the following options are available: +/// * RO - Read-Only. Reads the value of the key, but doesn't necessarily return it. +/// * RW - Read-Write. Modifies the data stored in the value of the key or its metadata. +/// * OW - Overwrite. Overwrites the data stored in the value of the key. +/// * RM - Deletes the key. +/// * ACCESS - Returns, copies or uses the user data from the value of the key. +/// * UPDATE - Updates data to the value, new value may depend on the old value. +/// * INSERT - Adds data to the value with no chance of modification or deletion of existing data. +/// * DELETE - Explicitly deletes some content from the value of the key. +/// * NOT_KEY - The key is not actually a key, but should be routed in cluster mode as if it was a key. +/// * INCOMPLETE - The keyspec might not point out all the keys it should cover. +/// * VARIABLE_FLAGS - Some keys might have different flags depending on arguments. +/// * begin_search - Represents how Redis should start looking for keys. +/// There are 2 possible options: +/// * Index - start looking for keys from a given position. +/// * Keyword - Search for a specific keyward and start looking for keys from this keyword +/// * FindKeys - After Redis finds the location from where it needs to start looking for keys, +/// Redis will start finding keys base on the information in this struct. +/// There are 2 possible options: +/// * Range - A tuple represent a range of `(last_key, steps, limit)`. +/// * last_key - Index of the last key relative to the result of the +/// begin search step. Can be negative, in which case it's not +/// relative. -1 indicates the last argument, -2 one before the +/// last and so on. +/// * steps - How many arguments should we skip after finding a +/// key, in order to find the next one. +/// * limit - If `lastkey` is -1, we use `limit` to stop the search +/// by a factor. 0 and 1 mean no limit. 2 means 1/2 of the +/// remaining args, 3 means 1/3, and so on. +/// * Keynum - A tuple of 3 elements `(keynumidx, firstkey, keystep)`. +/// * keynumidx - Index of the argument containing the number of +/// keys to come, relative to the result of the begin search step. +/// * firstkey - Index of the fist key relative to the result of the +/// begin search step. (Usually it's just after `keynumidx`, in +/// which case it should be set to `keynumidx + 1`.) +/// * keystep - How many arguments should we skip after finding a +/// key, in order to find the next one? +/// +/// Example: +/// The following example will register a command called `foo`. +/// ```rust,no_run,ignore +/// #[redis_command( +/// { +/// name: "foo", +/// arity: 3, +/// key_spec: [ +/// { +/// notes: "some notes", +/// flags: ["RW", "ACCESS"], +/// begin_search: Keyword(("foo", 1)), +/// find_keys: Range((1, 2, 3)), +/// } +/// ] +/// } +/// )] +/// fn test_command(_ctx: &Context, _args: Vec) -> RedisResult { +/// Ok(RedisValue::SimpleStringStatic("OK")) +/// } +/// ``` +/// +/// **Notice**, by default Redis does not validate the command spec. User should validate the command keys on the module command code. The command spec is used for validation on cluster so Redis can raise a cross slot error when needed. +#[proc_macro_attribute] +pub fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream { + redis_command::redis_command(attr, item) +} + #[proc_macro_attribute] pub fn role_changed_event_handler(_attr: TokenStream, item: TokenStream) -> TokenStream { let ast: ItemFn = match syn::parse(item) { diff --git a/redismodule-rs-macros/src/redis_command.rs b/redismodule-rs-macros/src/redis_command.rs new file mode 100644 index 00000000..896268dd --- /dev/null +++ b/redismodule-rs-macros/src/redis_command.rs @@ -0,0 +1,178 @@ +use proc_macro::TokenStream; +use proc_macro2::Ident; +use quote::quote; +use serde::Deserialize; +use serde_syn::{config, from_stream}; +use syn::{ + parse, + parse::{Parse, ParseStream}, + parse_macro_input, ItemFn, +}; + +#[derive(Debug, Deserialize)] +pub enum FindKeys { + Range((i32, i32, i32)), // (last_key, steps, limit) + Keynum((i32, i32, i32)), // (keynumidx, firstkey, keystep) +} + +#[derive(Debug, Deserialize)] +pub enum BeginSearch { + Index(i32), + Keyword((String, i32)), // (keyword, startfrom) +} + +#[derive(Debug, Deserialize)] +pub struct KeySpecArg { + notes: Option, + flags: Vec, + begin_search: BeginSearch, + find_keys: FindKeys, +} + +#[derive(Debug, Deserialize)] +struct Args { + name: String, + flags: Option, + summary: Option, + complexity: Option, + since: Option, + tips: Option, + arity: i64, + key_spec: Vec, +} + +impl Parse for Args { + fn parse(input: ParseStream) -> parse::Result { + from_stream(config::JSONY, &input) + } +} + +fn to_token_stream(s: Option) -> proc_macro2::TokenStream { + s.map(|v| quote! {Some(#v.to_owned())}) + .unwrap_or(quote! {None}) +} + +pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream { + let args = parse_macro_input!(attr as Args); + let func: ItemFn = match syn::parse(item) { + Ok(res) => res, + Err(e) => return e.to_compile_error().into(), + }; + + let original_function_name = func.sig.ident.clone(); + + let c_function_name = Ident::new( + &format!("_inner_{}", func.sig.ident.to_string()), + func.sig.ident.span(), + ); + + let get_command_info_function_name = Ident::new( + &format!("_inner_get_command_info_{}", func.sig.ident.to_string()), + func.sig.ident.span(), + ); + + let name_literal = args.name; + let flags_literal = to_token_stream(args.flags); + let summary_literal = to_token_stream(args.summary); + let complexity_literal = to_token_stream(args.complexity); + let since_literal = to_token_stream(args.since); + let tips_literal = to_token_stream(args.tips); + let arity_literal = args.arity; + let key_spec_notes: Vec<_> = args + .key_spec + .iter() + .map(|v| { + v.notes + .as_ref() + .map(|v| quote! {Some(#v.to_owned())}) + .unwrap_or(quote! {None}) + }) + .collect(); + + let key_spec_flags: Vec<_> = args + .key_spec + .iter() + .map(|v| { + let flags = &v.flags; + quote! { + vec![#(redis_module::commnads::KeySpecFlags::try_from(#flags)?, )*] + } + }) + .collect(); + + let key_spec_begin_search: Vec<_> = args + .key_spec + .iter() + .map(|v| match &v.begin_search { + BeginSearch::Index(i) => { + quote! { + redis_module::commnads::BeginSearch::Index(#i) + } + } + BeginSearch::Keyword((k, i)) => { + quote! { + redis_module::commnads::BeginSearch::Keyword((#k.to_owned(), #i)) + } + } + }) + .collect(); + + let key_spec_find_keys: Vec<_> = args + .key_spec + .iter() + .map(|v| match &v.find_keys { + FindKeys::Keynum((keynumidx, firstkey, keystep)) => { + quote! { + redis_module::commnads::FindKeys::Keynum((#keynumidx, #firstkey, #keystep)) + } + } + FindKeys::Range((last_key, steps, limit)) => { + quote! { + redis_module::commnads::FindKeys::Range((#last_key, #steps, #limit)) + } + } + }) + .collect(); + + let gen = quote! { + #func + + extern "C" fn #c_function_name( + ctx: *mut redis_module::raw::RedisModuleCtx, + argv: *mut *mut redis_module::raw::RedisModuleString, + argc: i32, + ) -> i32 { + let context = redis_module::Context::new(ctx); + + let args = redis_module::decode_args(ctx, argv, argc); + let response = #original_function_name(&context, args); + context.reply(response) as i32 + } + + #[linkme::distributed_slice(redis_module::commnads::COMMNADS_LIST)] + fn #get_command_info_function_name() -> Result { + let key_spec = vec![ + #( + redis_module::commnads::KeySpec::new( + #key_spec_notes, + #key_spec_flags.into(), + #key_spec_begin_search, + #key_spec_find_keys, + ), + )* + ]; + Ok(redis_module::commnads::CommandInfo::new( + #name_literal.to_owned(), + #flags_literal, + #summary_literal, + #complexity_literal, + #since_literal, + #tips_literal, + #arity_literal, + key_spec, + #c_function_name, + )) + } + }; + gen.into() +} diff --git a/src/context/commnads.rs b/src/context/commnads.rs new file mode 100644 index 00000000..22779853 --- /dev/null +++ b/src/context/commnads.rs @@ -0,0 +1,409 @@ +use crate::raw; +use crate::Context; +use crate::RedisError; +use crate::Status; +use bitflags::bitflags; +use linkme::distributed_slice; +use redis_module_macros_internals::redismodule_api; +use std::ffi::CString; +use std::mem::MaybeUninit; +use std::os::raw::c_int; +use std::ptr; + +const COMMNAD_INFO_VERSION: raw::RedisModuleCommandInfoVersion = + raw::RedisModuleCommandInfoVersion { + version: 1, + sizeof_historyentry: std::mem::size_of::(), + sizeof_keyspec: std::mem::size_of::(), + sizeof_arg: std::mem::size_of::(), + }; + +bitflags! { + /// Key spec flags + /// + /// The first four refer to what the command actually does with the value or + /// metadata of the key, and not necessarily the user data or how it affects + /// it. Each key-spec must have exactly one of these. Any operation + /// that's not distinctly deletion, overwrite or read-only would be marked as + /// RW. + /// + /// The next four refer to user data inside the value of the key, not the + /// metadata like LRU, type, cardinality. It refers to the logical operation + /// on the user's data (actual input strings or TTL), being + /// used/returned/copied/changed. It doesn't refer to modification or + /// returning of metadata (like type, count, presence of data). ACCESS can be + /// combined with one of the write operations INSERT, DELETE or UPDATE. Any + /// write that's not an INSERT or a DELETE would be UPDATE. + pub struct KeySpecFlags : u32 { + /// Read-Only. Reads the value of the key, but doesn't necessarily return it. + const RO = raw::REDISMODULE_CMD_KEY_RO; + + /// Read-Write. Modifies the data stored in the value of the key or its metadata. + const RW = raw::REDISMODULE_CMD_KEY_RW; + + /// Overwrite. Overwrites the data stored in the value of the key. + const OW = raw::REDISMODULE_CMD_KEY_OW; + + /// Deletes the key. + const RM = raw::REDISMODULE_CMD_KEY_RM; + + /// Returns, copies or uses the user data from the value of the key. + const ACCESS = raw::REDISMODULE_CMD_KEY_ACCESS; + + /// Updates data to the value, new value may depend on the old value. + const UPDATE = raw::REDISMODULE_CMD_KEY_UPDATE; + + /// Adds data to the value with no chance of modification or deletion of existing data. + const INSERT = raw::REDISMODULE_CMD_KEY_INSERT; + + /// Explicitly deletes some content from the value of the key. + const DELETE = raw::REDISMODULE_CMD_KEY_DELETE; + + /// The key is not actually a key, but should be routed in cluster mode as if it was a key. + const NOT_KEY = raw::REDISMODULE_CMD_KEY_NOT_KEY; + + /// The keyspec might not point out all the keys it should cover. + const INCOMPLETE = raw::REDISMODULE_CMD_KEY_INCOMPLETE; + + /// Some keys might have different flags depending on arguments. + const VARIABLE_FLAGS = raw::REDISMODULE_CMD_KEY_VARIABLE_FLAGS; + } +} + +impl TryFrom<&str> for KeySpecFlags { + type Error = RedisError; + fn try_from(value: &str) -> Result { + match value { + "RO" => Ok(KeySpecFlags::RO), + "RW" => Ok(KeySpecFlags::RW), + "OW" => Ok(KeySpecFlags::OW), + "RM" => Ok(KeySpecFlags::RM), + "ACCESS" => Ok(KeySpecFlags::ACCESS), + "UPDATE" => Ok(KeySpecFlags::UPDATE), + "INSERT" => Ok(KeySpecFlags::INSERT), + "DELETE" => Ok(KeySpecFlags::DELETE), + "NOT_KEY" => Ok(KeySpecFlags::NOT_KEY), + "INCOMPLETE" => Ok(KeySpecFlags::INCOMPLETE), + "VARIABLE_FLAGS" => Ok(KeySpecFlags::VARIABLE_FLAGS), + _ => Err(RedisError::String(format!( + "Value {value} is not a valid key spec flag." + ))), + } + } +} + +impl From> for KeySpecFlags { + fn from(value: Vec) -> Self { + value + .into_iter() + .fold(KeySpecFlags::empty(), |a, item| a | item) + } +} + +/// This struct represents how Redis should start looking for keys. +/// There are 2 possible options: +/// 1. Index - start looking for keys from a given position. +/// 2. Keyword - Search for a specific keyward and start looking for keys from this keyword +pub enum BeginSearch { + Index(i32), + Keyword((String, i32)), // (keyword, startfrom) +} + +impl BeginSearch { + fn get_redis_begin_search( + &self, + ) -> ( + raw::RedisModuleKeySpecBeginSearchType, + raw::RedisModuleCommandKeySpec__bindgen_ty_1, + ) { + match self { + BeginSearch::Index(i) => ( + raw::RedisModuleKeySpecBeginSearchType_REDISMODULE_KSPEC_BS_INDEX, + raw::RedisModuleCommandKeySpec__bindgen_ty_1 { + index: raw::RedisModuleCommandKeySpec__bindgen_ty_1__bindgen_ty_1 { pos: *i }, + }, + ), + BeginSearch::Keyword((k, i)) => { + let keyword = CString::new(k.as_str()).unwrap().into_raw(); + ( + raw::RedisModuleKeySpecBeginSearchType_REDISMODULE_KSPEC_BS_KEYWORD, + raw::RedisModuleCommandKeySpec__bindgen_ty_1 { + keyword: raw::RedisModuleCommandKeySpec__bindgen_ty_1__bindgen_ty_2 { + keyword, + startfrom: *i, + }, + }, + ) + } + } + } +} + +/// After Redis finds the location from where it needs to start looking for keys, +/// Redis will start finding keys base on the information in this struct. +/// There are 2 possible options: +/// 1. Range - A tuple represent a range of `(last_key, steps, limit)`. +/// 2. Keynum - A tuple of 3 elements `(keynumidx, firstkey, keystep)`. +/// Redis will consider the argument at `keynumidx` as an indicator +/// to the number of keys that will follow. Then it will start +/// from `firstkey` and jump each `keystep` to find the keys. +pub enum FindKeys { + Range((i32, i32, i32)), // (last_key, steps, limit) + Keynum((i32, i32, i32)), // (keynumidx, firstkey, keystep) +} + +impl FindKeys { + fn get_redis_find_keys( + &self, + ) -> ( + raw::RedisModuleKeySpecFindKeysType, + raw::RedisModuleCommandKeySpec__bindgen_ty_2, + ) { + match self { + FindKeys::Range((lastkey, keystep, limit)) => ( + raw::RedisModuleKeySpecFindKeysType_REDISMODULE_KSPEC_FK_RANGE, + raw::RedisModuleCommandKeySpec__bindgen_ty_2 { + range: raw::RedisModuleCommandKeySpec__bindgen_ty_2__bindgen_ty_1 { + lastkey: *lastkey, + keystep: *keystep, + limit: *limit, + }, + }, + ), + FindKeys::Keynum((keynumidx, firstkey, keystep)) => ( + raw::RedisModuleKeySpecFindKeysType_REDISMODULE_KSPEC_FK_KEYNUM, + raw::RedisModuleCommandKeySpec__bindgen_ty_2 { + keynum: raw::RedisModuleCommandKeySpec__bindgen_ty_2__bindgen_ty_2 { + keynumidx: *keynumidx, + firstkey: *firstkey, + keystep: *keystep, + }, + }, + ), + } + } +} + +pub struct KeySpec { + notes: Option, + flags: KeySpecFlags, + begin_search: BeginSearch, + find_keys: FindKeys, +} + +impl KeySpec { + pub fn new( + notes: Option, + flags: KeySpecFlags, + begin_search: BeginSearch, + find_keys: FindKeys, + ) -> KeySpec { + KeySpec { + notes, + flags, + begin_search, + find_keys, + } + } + fn get_redis_key_spec(&self) -> raw::RedisModuleCommandKeySpec { + let (begin_search_type, bs) = self.begin_search.get_redis_begin_search(); + let (find_keys_type, fk) = self.find_keys.get_redis_find_keys(); + raw::RedisModuleCommandKeySpec { + notes: self + .notes + .as_ref() + .map(|v| CString::new(v.as_str()).unwrap().into_raw()) + .unwrap_or(ptr::null_mut()), + flags: self.flags.bits() as u64, + begin_search_type, + bs, + find_keys_type, + fk, + } + } +} + +type CommnadCallback = + extern "C" fn(*mut raw::RedisModuleCtx, *mut *mut raw::RedisModuleString, i32) -> i32; + +/// A struct represent a CommandInfo +pub struct CommandInfo { + name: String, + flags: Option, + summary: Option, + complexity: Option, + since: Option, + tips: Option, + arity: i64, + key_spec: Vec, + callback: CommnadCallback, +} + +impl CommandInfo { + pub fn new( + name: String, + flags: Option, + summary: Option, + complexity: Option, + since: Option, + tips: Option, + arity: i64, + key_spec: Vec, + callback: CommnadCallback, + ) -> CommandInfo { + CommandInfo { + name, + flags, + summary, + complexity, + since, + tips, + arity, + key_spec, + callback, + } + } +} + +#[distributed_slice()] +pub static COMMNADS_LIST: [fn() -> Result] = [..]; + +pub fn get_redis_key_spec(key_spec: Vec) -> *mut raw::RedisModuleCommandKeySpec { + let mut redis_key_spec: Vec = key_spec + .into_iter() + .map(|v| v.get_redis_key_spec()) + .collect(); + let zerod: raw::RedisModuleCommandKeySpec = unsafe { MaybeUninit::zeroed().assume_init() }; + redis_key_spec.push(zerod); + let res = redis_key_spec.as_ptr(); + std::mem::forget(redis_key_spec); + res as *mut raw::RedisModuleCommandKeySpec +} + +redismodule_api! {[ + RedisModule_CreateCommand, + RedisModule_GetCommand, + RedisModule_SetCommandInfo, + ], + /// Register all the commands located on `COMMNADS_LIST`. + fn register_commands_internal(ctx: &Context) -> Result<(), RedisError> { + COMMNADS_LIST.iter().try_for_each(|command| { + let command_info = command()?; + let name: CString = CString::new(command_info.name.as_str()).unwrap(); + let flags = CString::new( + command_info + .flags + .as_ref() + .map(|v| v.as_str()) + .unwrap_or(""), + ) + .unwrap(); + + if unsafe { + RedisModule_CreateCommand( + ctx.ctx, + name.as_ptr(), + Some(command_info.callback), + flags.as_ptr(), + 0, + 0, + 0, + ) + } == raw::Status::Err as i32 + { + return Err(RedisError::String(format!( + "Failed register command {}.", + command_info.name + ))); + } + + // Register the extra data of the command + let command = unsafe { RedisModule_GetCommand(ctx.ctx, name.as_ptr()) }; + + if command.is_null() { + return Err(RedisError::String(format!( + "Failed finding command {} after registration.", + command_info.name + ))); + } + + let summary = command_info + .summary + .as_ref() + .map(|v| CString::new(v.as_str()).unwrap().into_raw()) + .unwrap_or(ptr::null_mut()); + let complexity = command_info + .complexity + .as_ref() + .map(|v| CString::new(v.as_str()).unwrap().into_raw()) + .unwrap_or(ptr::null_mut()); + let since = command_info + .since + .as_ref() + .map(|v| CString::new(v.as_str()).unwrap().into_raw()) + .unwrap_or(ptr::null_mut()); + let tips = command_info + .tips + .as_ref() + .map(|v| CString::new(v.as_str()).unwrap().into_raw()) + .unwrap_or(ptr::null_mut()); + + let redis_command_info = Box::into_raw(Box::new(raw::RedisModuleCommandInfo { + version: &COMMNAD_INFO_VERSION, + summary, + complexity, + since, + history: ptr::null_mut(), // currently we will not support history + tips, + arity: command_info.arity as c_int, + key_specs: get_redis_key_spec(command_info.key_spec), + args: ptr::null_mut(), + })); + + if unsafe { RedisModule_SetCommandInfo(command, redis_command_info) } == raw::Status::Err as i32 { + return Err(RedisError::String(format!( + "Failed setting info for command {}.", + command_info.name + ))); + } + + Ok(()) + }) + } +} + +#[cfg(any( + feature = "min-redis-compatibility-version-7-2", + feature = "min-redis-compatibility-version-7-0" +))] +pub fn register_commands(ctx: &Context) -> Status { + register_commands_internal(ctx).map_or_else( + |e| { + ctx.log_warning(&e.to_string()); + Status::Err + }, + |_| Status::Ok, + ) +} + +#[cfg(any( + feature = "min-redis-compatibility-version-6-2", + feature = "min-redis-compatibility-version-6-0" +))] +pub fn register_commands(ctx: &Context) -> Status { + register_commands_internal(ctx).map_or_else( + |e| { + ctx.log_warning(&e.to_string()); + Status::Err + }, + |v| { + v.map_or_else( + |e| { + ctx.log_warning(&e.to_string()); + Status::Err + }, + |_| Status::Ok, + ) + }, + ) +} diff --git a/src/context/mod.rs b/src/context/mod.rs index 35604e0c..a3b99d91 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -22,6 +22,7 @@ mod timer; pub mod blocked; pub mod call_reply; +pub mod commnads; pub mod info; pub mod keys_cursor; pub mod server_events; diff --git a/src/lib.rs b/src/lib.rs index b6ca34bf..656d0ae1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,6 +31,7 @@ pub use crate::raw::NotifyEvent; pub use crate::configuration::ConfigurationValue; pub use crate::configuration::EnumConfigurationValue; pub use crate::context::call_reply::{CallReply, CallResult, ErrorReply}; +pub use crate::context::commnads; pub use crate::context::keys_cursor::KeysCursor; pub use crate::context::server_events; pub use crate::context::AclPermissions; diff --git a/src/macros.rs b/src/macros.rs index 27f4a721..6348a2c7 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -224,6 +224,10 @@ macro_rules! redis_module { $crate::redis_command!(ctx, $name, $command, $flags, $firstkey, $lastkey, $keystep); )* + if $crate::commnads::register_commands(&context) == raw::Status::Err { + return raw::Status::Err as c_int; + } + $( $( $crate::redis_event_handler!(ctx, $(raw::NotifyEvent::$event_type |)+ raw::NotifyEvent::empty(), $event_handler); diff --git a/tests/integration.rs b/tests/integration.rs index 46e31ead..65ce9399 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -499,3 +499,42 @@ fn test_response() -> Result<()> { Ok(()) } + +#[test] +fn test_command_proc_macro() -> Result<()> { + let port: u16 = 6497; + let _guards = vec![start_redis_server_with_module("proc_macro_commands", 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("COMMAND") + .arg(&["GETKEYS", "classic_keys", "x", "foo", "y", "bar"]) + .query(&mut con) + .with_context(|| "failed to run string.set")?; + + assert_eq!(&res, &["x", "y"]); + + let res: Vec = redis::cmd("COMMAND") + .arg(&["GETKEYS", "keyword_keys", "foo", "x", "1", "y", "2"]) + .query(&mut con) + .with_context(|| "failed to run string.set")?; + + assert_eq!(&res, &["x", "y"]); + + let res: Vec = redis::cmd("COMMAND") + .arg(&["GETKEYS", "num_keys", "3", "x", "y", "z", "foo", "bar"]) + .query(&mut con) + .with_context(|| "failed to run string.set")?; + + assert_eq!(&res, &["x", "y", "z"]); + + let res: Vec = redis::cmd("COMMAND") + .arg(&["GETKEYS", "num_keys", "0", "foo", "bar"]) + .query(&mut con) + .with_context(|| "failed to run string.set")?; + + assert!(res.is_empty()); + + Ok(()) +} From 13f8f02e588b546f13c76190864cd005064c0848 Mon Sep 17 00:00:00 2001 From: meir Date: Sat, 13 May 2023 12:07:45 +0300 Subject: [PATCH 02/10] Review fixes. --- examples/proc_macro_commands.rs | 26 +++---- .../src/{redis_command.rs => command.rs} | 72 ++++++++++++++----- redismodule-rs-macros/src/lib.rs | 8 +-- src/context/{commnads.rs => commands.rs} | 16 ++--- src/context/mod.rs | 2 +- src/lib.rs | 2 +- src/macros.rs | 2 +- 7 files changed, 82 insertions(+), 46 deletions(-) rename redismodule-rs-macros/src/{redis_command.rs => command.rs} (69%) rename src/context/{commnads.rs => commands.rs} (96%) diff --git a/examples/proc_macro_commands.rs b/examples/proc_macro_commands.rs index 73cb50a2..4cbbe1e3 100644 --- a/examples/proc_macro_commands.rs +++ b/examples/proc_macro_commands.rs @@ -1,7 +1,7 @@ use redis_module::{redis_module, Context, RedisResult, RedisString, RedisValue}; -use redis_module_macros::redis_command; +use redis_module_macros::command; -#[redis_command( +#[command( { name: "classic_keys", flags: "readonly", @@ -9,9 +9,9 @@ use redis_module_macros::redis_command; key_spec: [ { notes: "test command that define all the arguments at even possition as keys", - flags: ["RO", "ACCESS"], - begin_search: Index(1), - find_keys: Range((-1, 2, 0)), + flags: ["READ_ONLY", "ACCESS"], + begin_search: Index({ index : 1 }), + find_keys: Range({ last_key :- 1, steps : 2, limit : 0 }), } ] } @@ -20,7 +20,7 @@ fn classic_keys(_ctx: &Context, _args: Vec) -> RedisResult { Ok(RedisValue::SimpleStringStatic("OK")) } -#[redis_command( +#[command( { name: "keyword_keys", flags: "readonly", @@ -28,9 +28,9 @@ fn classic_keys(_ctx: &Context, _args: Vec) -> RedisResult { key_spec: [ { notes: "test command that define all the arguments at even possition as keys", - flags: ["RO", "ACCESS"], - begin_search: Keyword(("foo", 1)), - find_keys: Range((-1, 2, 0)), + flags: ["READ_ONLY", "ACCESS"], + begin_search: Keyword({ keyword : "foo", startfrom : 1 }), + find_keys: Range({ last_key :- 1, steps : 2, limit : 0 }), } ] } @@ -39,7 +39,7 @@ fn keyword_keys(_ctx: &Context, _args: Vec) -> RedisResult { Ok(RedisValue::SimpleStringStatic("OK")) } -#[redis_command( +#[command( { name: "num_keys", flags: "readonly no-mandatory-keys", @@ -47,9 +47,9 @@ fn keyword_keys(_ctx: &Context, _args: Vec) -> RedisResult { key_spec: [ { notes: "test command that define all the arguments at even possition as keys", - flags: ["RO", "ACCESS"], - begin_search: Index(1), - find_keys: Keynum((0, 1, 1)), + flags: ["READ_ONLY", "ACCESS"], + begin_search: Index({ index : 1 }), + find_keys: Keynum({ key_num_idx : 0, first_key : 1, key_step : 1 }), } ] } diff --git a/redismodule-rs-macros/src/redis_command.rs b/redismodule-rs-macros/src/command.rs similarity index 69% rename from redismodule-rs-macros/src/redis_command.rs rename to redismodule-rs-macros/src/command.rs index 896268dd..69089ef1 100644 --- a/redismodule-rs-macros/src/redis_command.rs +++ b/redismodule-rs-macros/src/command.rs @@ -9,16 +9,41 @@ use syn::{ parse_macro_input, ItemFn, }; +#[derive(Debug, Deserialize)] +pub struct FindKeysRange { + last_key: i32, + steps: i32, + limit: i32, +} + +#[derive(Debug, Deserialize)] +pub struct FindKeysNum { + key_num_idx: i32, + first_key: i32, + key_step: i32, +} + #[derive(Debug, Deserialize)] pub enum FindKeys { - Range((i32, i32, i32)), // (last_key, steps, limit) - Keynum((i32, i32, i32)), // (keynumidx, firstkey, keystep) + Range(FindKeysRange), + Keynum(FindKeysNum), +} + +#[derive(Debug, Deserialize)] +pub struct BeginSearchIndex { + index: i32, +} + +#[derive(Debug, Deserialize)] +pub struct BeginSearchKeyword { + keyword: String, + startfrom: i32, } #[derive(Debug, Deserialize)] pub enum BeginSearch { - Index(i32), - Keyword((String, i32)), // (keyword, startfrom) + Index(BeginSearchIndex), + Keyword(BeginSearchKeyword), // (keyword, startfrom) } #[derive(Debug, Deserialize)] @@ -31,7 +56,7 @@ pub struct KeySpecArg { #[derive(Debug, Deserialize)] struct Args { - name: String, + name: Option, flags: Option, summary: Option, complexity: Option, @@ -71,7 +96,9 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream func.sig.ident.span(), ); - let name_literal = args.name; + let name_literal = args + .name + .unwrap_or_else(|| original_function_name.to_string()); let flags_literal = to_token_stream(args.flags); let summary_literal = to_token_stream(args.summary); let complexity_literal = to_token_stream(args.complexity); @@ -95,7 +122,7 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream .map(|v| { let flags = &v.flags; quote! { - vec![#(redis_module::commnads::KeySpecFlags::try_from(#flags)?, )*] + vec![#(redis_module::commands::KeySpecFlags::try_from(#flags)?, )*] } }) .collect(); @@ -105,13 +132,16 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream .iter() .map(|v| match &v.begin_search { BeginSearch::Index(i) => { + let i = i.index; quote! { - redis_module::commnads::BeginSearch::Index(#i) + redis_module::commands::BeginSearch::Index(#i) } } - BeginSearch::Keyword((k, i)) => { + BeginSearch::Keyword(begin_search_keyword) => { + let k = begin_search_keyword.keyword.as_str(); + let i = begin_search_keyword.startfrom; quote! { - redis_module::commnads::BeginSearch::Keyword((#k.to_owned(), #i)) + redis_module::commands::BeginSearch::Keyword((#k.to_owned(), #i)) } } }) @@ -121,14 +151,20 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream .key_spec .iter() .map(|v| match &v.find_keys { - FindKeys::Keynum((keynumidx, firstkey, keystep)) => { + FindKeys::Keynum(find_keys_num) => { + let keynumidx = find_keys_num.key_num_idx; + let firstkey = find_keys_num.first_key; + let keystep = find_keys_num.key_step; quote! { - redis_module::commnads::FindKeys::Keynum((#keynumidx, #firstkey, #keystep)) + redis_module::commands::FindKeys::Keynum((#keynumidx, #firstkey, #keystep)) } } - FindKeys::Range((last_key, steps, limit)) => { + FindKeys::Range(find_keys_range) => { + let last_key = find_keys_range.last_key; + let steps = find_keys_range.steps; + let limit = find_keys_range.limit; quote! { - redis_module::commnads::FindKeys::Range((#last_key, #steps, #limit)) + redis_module::commands::FindKeys::Range((#last_key, #steps, #limit)) } } }) @@ -149,11 +185,11 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream context.reply(response) as i32 } - #[linkme::distributed_slice(redis_module::commnads::COMMNADS_LIST)] - fn #get_command_info_function_name() -> Result { + #[linkme::distributed_slice(redis_module::commands::COMMNADS_LIST)] + fn #get_command_info_function_name() -> Result { let key_spec = vec![ #( - redis_module::commnads::KeySpec::new( + redis_module::commands::KeySpec::new( #key_spec_notes, #key_spec_flags.into(), #key_spec_begin_search, @@ -161,7 +197,7 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream ), )* ]; - Ok(redis_module::commnads::CommandInfo::new( + Ok(redis_module::commands::CommandInfo::new( #name_literal.to_owned(), #flags_literal, #summary_literal, diff --git a/redismodule-rs-macros/src/lib.rs b/redismodule-rs-macros/src/lib.rs index e15181b5..d9f3cc17 100644 --- a/redismodule-rs-macros/src/lib.rs +++ b/redismodule-rs-macros/src/lib.rs @@ -2,7 +2,7 @@ use proc_macro::TokenStream; use quote::quote; use syn::ItemFn; -mod redis_command; +mod command; /// This proc macro allow to specify that the follow function is a Redis command. /// The macro accept the following arguments that discribe the command properties: @@ -57,7 +57,7 @@ mod redis_command; /// Example: /// The following example will register a command called `foo`. /// ```rust,no_run,ignore -/// #[redis_command( +/// #[command( /// { /// name: "foo", /// arity: 3, @@ -78,8 +78,8 @@ mod redis_command; /// /// **Notice**, by default Redis does not validate the command spec. User should validate the command keys on the module command code. The command spec is used for validation on cluster so Redis can raise a cross slot error when needed. #[proc_macro_attribute] -pub fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream { - redis_command::redis_command(attr, item) +pub fn command(attr: TokenStream, item: TokenStream) -> TokenStream { + command::redis_command(attr, item) } #[proc_macro_attribute] diff --git a/src/context/commnads.rs b/src/context/commands.rs similarity index 96% rename from src/context/commnads.rs rename to src/context/commands.rs index 22779853..1df35796 100644 --- a/src/context/commnads.rs +++ b/src/context/commands.rs @@ -36,16 +36,16 @@ bitflags! { /// write that's not an INSERT or a DELETE would be UPDATE. pub struct KeySpecFlags : u32 { /// Read-Only. Reads the value of the key, but doesn't necessarily return it. - const RO = raw::REDISMODULE_CMD_KEY_RO; + const READ_ONLY = raw::REDISMODULE_CMD_KEY_RO; /// Read-Write. Modifies the data stored in the value of the key or its metadata. - const RW = raw::REDISMODULE_CMD_KEY_RW; + const READ_WRITE = raw::REDISMODULE_CMD_KEY_RW; /// Overwrite. Overwrites the data stored in the value of the key. - const OW = raw::REDISMODULE_CMD_KEY_OW; + const OVERWRITE = raw::REDISMODULE_CMD_KEY_OW; /// Deletes the key. - const RM = raw::REDISMODULE_CMD_KEY_RM; + const REMOVE = raw::REDISMODULE_CMD_KEY_RM; /// Returns, copies or uses the user data from the value of the key. const ACCESS = raw::REDISMODULE_CMD_KEY_ACCESS; @@ -74,10 +74,10 @@ impl TryFrom<&str> for KeySpecFlags { type Error = RedisError; fn try_from(value: &str) -> Result { match value { - "RO" => Ok(KeySpecFlags::RO), - "RW" => Ok(KeySpecFlags::RW), - "OW" => Ok(KeySpecFlags::OW), - "RM" => Ok(KeySpecFlags::RM), + "READ_ONLY" => Ok(KeySpecFlags::READ_ONLY), + "READ_WRITE" => Ok(KeySpecFlags::READ_WRITE), + "OVERWRITE" => Ok(KeySpecFlags::OVERWRITE), + "REMOVE" => Ok(KeySpecFlags::REMOVE), "ACCESS" => Ok(KeySpecFlags::ACCESS), "UPDATE" => Ok(KeySpecFlags::UPDATE), "INSERT" => Ok(KeySpecFlags::INSERT), diff --git a/src/context/mod.rs b/src/context/mod.rs index b6886e99..d5ac0aa8 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -23,7 +23,7 @@ mod timer; pub mod blocked; pub mod call_reply; -pub mod commnads; +pub mod commands; pub mod info; pub mod keys_cursor; pub mod server_events; diff --git a/src/lib.rs b/src/lib.rs index 6d2a6d65..7abe5b96 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -28,7 +28,7 @@ pub use crate::raw::NotifyEvent; pub use crate::configuration::ConfigurationValue; pub use crate::configuration::EnumConfigurationValue; pub use crate::context::call_reply::{CallReply, CallResult, ErrorReply}; -pub use crate::context::commnads; +pub use crate::context::commands; pub use crate::context::keys_cursor::KeysCursor; pub use crate::context::server_events; pub use crate::context::AclPermissions; diff --git a/src/macros.rs b/src/macros.rs index a5b917ed..f3eed320 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -227,7 +227,7 @@ macro_rules! redis_module { $crate::redis_command!(ctx, $name, $command, $flags, $firstkey, $lastkey, $keystep); )* - if $crate::commnads::register_commands(&context) == raw::Status::Err { + if $crate::commands::register_commands(&context) == raw::Status::Err { return raw::Status::Err as c_int; } From 4e87b57dbd0d83c731184311a7ed272ae746a7b4 Mon Sep 17 00:00:00 2001 From: meir Date: Sun, 14 May 2023 09:13:22 +0300 Subject: [PATCH 03/10] Review fixes --- examples/proc_macro_commands.rs | 6 +-- redismodule-rs-macros-internals/src/lib.rs | 2 +- redismodule-rs-macros/src/command.rs | 58 +++++++++++++++++++++- src/context/commands.rs | 51 ++++++++++--------- src/context/mod.rs | 4 +- 5 files changed, 90 insertions(+), 31 deletions(-) diff --git a/examples/proc_macro_commands.rs b/examples/proc_macro_commands.rs index 4cbbe1e3..089227cb 100644 --- a/examples/proc_macro_commands.rs +++ b/examples/proc_macro_commands.rs @@ -9,7 +9,7 @@ use redis_module_macros::command; key_spec: [ { notes: "test command that define all the arguments at even possition as keys", - flags: ["READ_ONLY", "ACCESS"], + flags: [ReadOnly, Access], begin_search: Index({ index : 1 }), find_keys: Range({ last_key :- 1, steps : 2, limit : 0 }), } @@ -28,7 +28,7 @@ fn classic_keys(_ctx: &Context, _args: Vec) -> RedisResult { key_spec: [ { notes: "test command that define all the arguments at even possition as keys", - flags: ["READ_ONLY", "ACCESS"], + flags: [ReadOnly, Access], begin_search: Keyword({ keyword : "foo", startfrom : 1 }), find_keys: Range({ last_key :- 1, steps : 2, limit : 0 }), } @@ -47,7 +47,7 @@ fn keyword_keys(_ctx: &Context, _args: Vec) -> RedisResult { key_spec: [ { notes: "test command that define all the arguments at even possition as keys", - flags: ["READ_ONLY", "ACCESS"], + flags: [ReadOnly, Access], begin_search: Index({ index : 1 }), find_keys: Keynum({ key_num_idx : 0, first_key : 1, key_step : 1 }), } diff --git a/redismodule-rs-macros-internals/src/lib.rs b/redismodule-rs-macros-internals/src/lib.rs index 771af114..befb6833 100644 --- a/redismodule-rs-macros-internals/src/lib.rs +++ b/redismodule-rs-macros-internals/src/lib.rs @@ -63,7 +63,7 @@ impl Parse for Args { /// ); /// ``` #[proc_macro] -pub fn redismodule_api(item: TokenStream) -> TokenStream { +pub fn api(item: TokenStream) -> TokenStream { let args = parse_macro_input!(item as Args); let minimum_require_version = args.requested_apis diff --git a/redismodule-rs-macros/src/command.rs b/redismodule-rs-macros/src/command.rs index 69089ef1..ff217bc3 100644 --- a/redismodule-rs-macros/src/command.rs +++ b/redismodule-rs-macros/src/command.rs @@ -9,6 +9,60 @@ use syn::{ parse_macro_input, ItemFn, }; +#[derive(Debug, Deserialize)] +pub enum RedisCommandKeySpecFlags { + /// Read-Only. Reads the value of the key, but doesn't necessarily return it. + ReadOnly, + + /// Read-Write. Modifies the data stored in the value of the key or its metadata. + ReadWrite, + + /// Overwrite. Overwrites the data stored in the value of the key. + Overwrite, + + /// Deletes the key. + Remove, + + /// Returns, copies or uses the user data from the value of the key. + Access, + + /// Updates data to the value, new value may depend on the old value. + Update, + + /// Adds data to the value with no chance of modification or deletion of existing data. + Insert, + + /// Explicitly deletes some content from the value of the key. + Delete, + + /// The key is not actually a key, but should be routed in cluster mode as if it was a key. + NotKey, + + /// The keyspec might not point out all the keys it should cover. + Incomplete, + + /// Some keys might have different flags depending on arguments. + VariableFlag, +} + +impl From<&RedisCommandKeySpecFlags> for &'static str { + fn from(value: &RedisCommandKeySpecFlags) -> Self { + match value { + RedisCommandKeySpecFlags::ReadOnly => "READ_ONLY", + RedisCommandKeySpecFlags::ReadWrite => "READ_WRITE", + RedisCommandKeySpecFlags::Overwrite => "OVERWRITE", + RedisCommandKeySpecFlags::Remove => "REMOVE", + RedisCommandKeySpecFlags::Access => "ACCESS", + RedisCommandKeySpecFlags::Update => "UPDATE", + RedisCommandKeySpecFlags::Insert => "INSERT", + RedisCommandKeySpecFlags::Delete => "DELETE", + RedisCommandKeySpecFlags::NotKey => "NOT_KEY", + RedisCommandKeySpecFlags::Incomplete => "INCOMPLETE", + RedisCommandKeySpecFlags::VariableFlag => "VARIABLE_FLAGS", + } + } +} + #[derive(Debug, Deserialize)] pub struct FindKeysRange { last_key: i32, @@ -49,7 +103,7 @@ pub enum BeginSearch { #[derive(Debug, Deserialize)] pub struct KeySpecArg { notes: Option, - flags: Vec, + flags: Vec, begin_search: BeginSearch, find_keys: FindKeys, } @@ -120,7 +174,7 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream .key_spec .iter() .map(|v| { - let flags = &v.flags; + let flags: Vec<&'static str> = v.flags.iter().map(|v| v.into()).collect(); quote! { vec![#(redis_module::commands::KeySpecFlags::try_from(#flags)?, )*] } diff --git a/src/context/commands.rs b/src/context/commands.rs index 1df35796..11aba730 100644 --- a/src/context/commands.rs +++ b/src/context/commands.rs @@ -4,7 +4,7 @@ use crate::RedisError; use crate::Status; use bitflags::bitflags; use linkme::distributed_slice; -use redis_module_macros_internals::redismodule_api; +use redis_module_macros_internals::api; use std::ffi::CString; use std::mem::MaybeUninit; use std::os::raw::c_int; @@ -109,14 +109,14 @@ pub enum BeginSearch { Keyword((String, i32)), // (keyword, startfrom) } -impl BeginSearch { - fn get_redis_begin_search( - &self, - ) -> ( +impl From<&BeginSearch> + for ( raw::RedisModuleKeySpecBeginSearchType, raw::RedisModuleCommandKeySpec__bindgen_ty_1, - ) { - match self { + ) +{ + fn from(value: &BeginSearch) -> Self { + match value { BeginSearch::Index(i) => ( raw::RedisModuleKeySpecBeginSearchType_REDISMODULE_KSPEC_BS_INDEX, raw::RedisModuleCommandKeySpec__bindgen_ty_1 { @@ -152,14 +152,14 @@ pub enum FindKeys { Keynum((i32, i32, i32)), // (keynumidx, firstkey, keystep) } -impl FindKeys { - fn get_redis_find_keys( - &self, - ) -> ( +impl From<&FindKeys> + for ( raw::RedisModuleKeySpecFindKeysType, raw::RedisModuleCommandKeySpec__bindgen_ty_2, - ) { - match self { + ) +{ + fn from(value: &FindKeys) -> Self { + match value { FindKeys::Range((lastkey, keystep, limit)) => ( raw::RedisModuleKeySpecFindKeysType_REDISMODULE_KSPEC_FK_RANGE, raw::RedisModuleCommandKeySpec__bindgen_ty_2 { @@ -184,6 +184,10 @@ impl FindKeys { } } +/// A struct that specify how to find keys from a command. +/// It is devided into 2 parts: +/// 1. begin_search - indicate how to find the first command argument from where to start searching for keys. +/// 2. find_keys - the methose to use in order to find the keys. pub struct KeySpec { notes: Option, flags: KeySpecFlags, @@ -205,16 +209,19 @@ impl KeySpec { find_keys, } } - fn get_redis_key_spec(&self) -> raw::RedisModuleCommandKeySpec { - let (begin_search_type, bs) = self.begin_search.get_redis_begin_search(); - let (find_keys_type, fk) = self.find_keys.get_redis_find_keys(); +} + +impl From<&KeySpec> for raw::RedisModuleCommandKeySpec { + fn from(value: &KeySpec) -> Self { + let (begin_search_type, bs) = (&value.begin_search).into(); + let (find_keys_type, fk) = (&value.find_keys).into(); raw::RedisModuleCommandKeySpec { - notes: self + notes: value .notes .as_ref() .map(|v| CString::new(v.as_str()).unwrap().into_raw()) .unwrap_or(ptr::null_mut()), - flags: self.flags.bits() as u64, + flags: value.flags.bits() as u64, begin_search_type, bs, find_keys_type, @@ -269,10 +276,8 @@ impl CommandInfo { pub static COMMNADS_LIST: [fn() -> Result] = [..]; pub fn get_redis_key_spec(key_spec: Vec) -> *mut raw::RedisModuleCommandKeySpec { - let mut redis_key_spec: Vec = key_spec - .into_iter() - .map(|v| v.get_redis_key_spec()) - .collect(); + let mut redis_key_spec: Vec = + key_spec.into_iter().map(|v| (&v).into()).collect(); let zerod: raw::RedisModuleCommandKeySpec = unsafe { MaybeUninit::zeroed().assume_init() }; redis_key_spec.push(zerod); let res = redis_key_spec.as_ptr(); @@ -280,7 +285,7 @@ pub fn get_redis_key_spec(key_spec: Vec) -> *mut raw::RedisModuleComman res as *mut raw::RedisModuleCommandKeySpec } -redismodule_api! {[ +api! {[ RedisModule_CreateCommand, RedisModule_GetCommand, RedisModule_SetCommandInfo, diff --git a/src/context/mod.rs b/src/context/mod.rs index d5ac0aa8..e4c47417 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -1,5 +1,5 @@ use bitflags::bitflags; -use redis_module_macros_internals::redismodule_api; +use redis_module_macros_internals::api; use std::ffi::CString; use std::os::raw::c_void; use std::os::raw::{c_char, c_int, c_long, c_longlong}; @@ -696,7 +696,7 @@ impl Context { acl_permission_result.map_err(|_e| RedisError::Str("User does not have permissions on key")) } - redismodule_api!( + api!( [RedisModule_AddPostNotificationJob], /// When running inside a key space notification callback, it is dangerous and highly discouraged to perform any write /// operation. In order to still perform write actions in this scenario, Redis provides this API ([add_post_notification_job]) From 45f32e109c0df806cb29b918132646979853e618 Mon Sep 17 00:00:00 2001 From: meir Date: Sun, 14 May 2023 09:22:51 +0300 Subject: [PATCH 04/10] Review fixes --- redismodule-rs-macros/src/command.rs | 4 ++-- redismodule-rs-macros/src/lib.rs | 24 ++++++++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/redismodule-rs-macros/src/command.rs b/redismodule-rs-macros/src/command.rs index ff217bc3..c6867b66 100644 --- a/redismodule-rs-macros/src/command.rs +++ b/redismodule-rs-macros/src/command.rs @@ -42,7 +42,7 @@ pub enum RedisCommandKeySpecFlags { Incomplete, /// Some keys might have different flags depending on arguments. - VariableFlag, + VariableFlags, } impl From<&RedisCommandKeySpecFlags> for &'static str { @@ -58,7 +58,7 @@ impl From<&RedisCommandKeySpecFlags> for &'static str { RedisCommandKeySpecFlags::Delete => "DELETE", RedisCommandKeySpecFlags::NotKey => "NOT_KEY", RedisCommandKeySpecFlags::Incomplete => "INCOMPLETE", - RedisCommandKeySpecFlags::VariableFlag => "VARIABLE_FLAGS", + RedisCommandKeySpecFlags::VariableFlags => "VARIABLE_FLAGS", } } } diff --git a/redismodule-rs-macros/src/lib.rs b/redismodule-rs-macros/src/lib.rs index d9f3cc17..59fc4df8 100644 --- a/redismodule-rs-macros/src/lib.rs +++ b/redismodule-rs-macros/src/lib.rs @@ -6,7 +6,7 @@ mod command; /// This proc macro allow to specify that the follow function is a Redis command. /// The macro accept the following arguments that discribe the command properties: -/// * name - The command name, +/// * name (optional) - The command name. in case not given, the function name will be taken. /// * flags (optional) - Command flags such as `readonly`, for the full list please refer to https://redis.io/docs/reference/modules/modules-api-ref/#redismodule_createcommand /// * summary (optional) - Command summary /// * complexity (optional) - Command compexity @@ -17,17 +17,17 @@ mod command; /// * key_spec - A list of specs representing how to find the keys that the command might touch. the following options are available: /// * notes (optional) - Some note about the key spec. /// * flags - List of flags reprenting how the keys are accessed, the following options are available: -/// * RO - Read-Only. Reads the value of the key, but doesn't necessarily return it. -/// * RW - Read-Write. Modifies the data stored in the value of the key or its metadata. -/// * OW - Overwrite. Overwrites the data stored in the value of the key. -/// * RM - Deletes the key. -/// * ACCESS - Returns, copies or uses the user data from the value of the key. -/// * UPDATE - Updates data to the value, new value may depend on the old value. -/// * INSERT - Adds data to the value with no chance of modification or deletion of existing data. -/// * DELETE - Explicitly deletes some content from the value of the key. -/// * NOT_KEY - The key is not actually a key, but should be routed in cluster mode as if it was a key. -/// * INCOMPLETE - The keyspec might not point out all the keys it should cover. -/// * VARIABLE_FLAGS - Some keys might have different flags depending on arguments. +/// * Readonly - Read-Only. Reads the value of the key, but doesn't necessarily return it. +/// * ReadWrite - Read-Write. Modifies the data stored in the value of the key or its metadata. +/// * Overwrite - Overwrite. Overwrites the data stored in the value of the key. +/// * Remove - Deletes the key. +/// * Access - Returns, copies or uses the user data from the value of the key. +/// * Update - Updates data to the value, new value may depend on the old value. +/// * Insert - Adds data to the value with no chance of modification or deletion of existing data. +/// * Delete - Explicitly deletes some content from the value of the key. +/// * NotKey - The key is not actually a key, but should be routed in cluster mode as if it was a key. +/// * Incomplete - The keyspec might not point out all the keys it should cover. +/// * VariableFlags - Some keys might have different flags depending on arguments. /// * begin_search - Represents how Redis should start looking for keys. /// There are 2 possible options: /// * Index - start looking for keys from a given position. From c0a9445038c2470365cab20285e9e252b4ae8e5c Mon Sep 17 00:00:00 2001 From: meir Date: Sun, 14 May 2023 09:28:33 +0300 Subject: [PATCH 05/10] Docs fixes --- redismodule-rs-macros/src/lib.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/redismodule-rs-macros/src/lib.rs b/redismodule-rs-macros/src/lib.rs index 59fc4df8..b932e587 100644 --- a/redismodule-rs-macros/src/lib.rs +++ b/redismodule-rs-macros/src/lib.rs @@ -35,7 +35,7 @@ mod command; /// * FindKeys - After Redis finds the location from where it needs to start looking for keys, /// Redis will start finding keys base on the information in this struct. /// There are 2 possible options: -/// * Range - A tuple represent a range of `(last_key, steps, limit)`. +/// * Range - An object of three element `last_key`, `steps`, `limit`. /// * last_key - Index of the last key relative to the result of the /// begin search step. Can be negative, in which case it's not /// relative. -1 indicates the last argument, -2 one before the @@ -45,7 +45,7 @@ mod command; /// * limit - If `lastkey` is -1, we use `limit` to stop the search /// by a factor. 0 and 1 mean no limit. 2 means 1/2 of the /// remaining args, 3 means 1/3, and so on. -/// * Keynum - A tuple of 3 elements `(keynumidx, firstkey, keystep)`. +/// * Keynum - An object of 3 elements `keynumidx`, `firstkey`, `keystep`. /// * keynumidx - Index of the argument containing the number of /// keys to come, relative to the result of the begin search step. /// * firstkey - Index of the fist key relative to the result of the @@ -59,14 +59,15 @@ mod command; /// ```rust,no_run,ignore /// #[command( /// { -/// name: "foo", -/// arity: 3, +/// name: "test", +/// flags: "readonly", +/// arity: -2, /// key_spec: [ /// { -/// notes: "some notes", -/// flags: ["RW", "ACCESS"], -/// begin_search: Keyword(("foo", 1)), -/// find_keys: Range((1, 2, 3)), +/// notes: "test command that define all the arguments at even possition as keys", +/// flags: [ReadOnly, Access], +/// begin_search: Keyword({ keyword : "foo", startfrom : 1 }), +/// find_keys: Range({ last_key :- 1, steps : 2, limit : 0 }), /// } /// ] /// } From 4637e68ef6aac35670e76b959de6e4f7728e322b Mon Sep 17 00:00:00 2001 From: meir Date: Mon, 15 May 2023 12:22:32 +0300 Subject: [PATCH 06/10] Review fixes --- examples/proc_macro_commands.rs | 6 +- redismodule-rs-macros/src/command.rs | 114 ++++++++++++++++++++++++++- 2 files changed, 114 insertions(+), 6 deletions(-) diff --git a/examples/proc_macro_commands.rs b/examples/proc_macro_commands.rs index 089227cb..0f2b7d0e 100644 --- a/examples/proc_macro_commands.rs +++ b/examples/proc_macro_commands.rs @@ -4,7 +4,7 @@ use redis_module_macros::command; #[command( { name: "classic_keys", - flags: "readonly", + flags: [ReadOnly], arity: -2, key_spec: [ { @@ -23,7 +23,7 @@ fn classic_keys(_ctx: &Context, _args: Vec) -> RedisResult { #[command( { name: "keyword_keys", - flags: "readonly", + flags: [ReadOnly], arity: -2, key_spec: [ { @@ -42,7 +42,7 @@ fn keyword_keys(_ctx: &Context, _args: Vec) -> RedisResult { #[command( { name: "num_keys", - flags: "readonly no-mandatory-keys", + flags: [ReadOnly, NoMandatoryKeys], arity: -2, key_spec: [ { diff --git a/redismodule-rs-macros/src/command.rs b/redismodule-rs-macros/src/command.rs index c6867b66..4f6ab064 100644 --- a/redismodule-rs-macros/src/command.rs +++ b/redismodule-rs-macros/src/command.rs @@ -9,6 +9,106 @@ use syn::{ parse_macro_input, ItemFn, }; +#[derive(Debug, Deserialize)] +pub enum RedisCommandFlags { + /// The command may modify the data set (it may also read from it). + Write, + + /// The command returns data from keys but never writes. + ReadOnly, + + /// The command is an administrative command (may change replication or perform similar tasks). + Admin, + + /// The command may use additional memory and should be denied during out of memory conditions. + DenyOOM, + + /// Don't allow this command in Lua scripts. + DenyScript, + + /// Allow this command while the server is loading data. Only commands not interacting with the data set + /// should be allowed to run in this mode. If not sure don't use this flag. + AllowLoading, + + /// The command publishes things on Pub/Sub channels. + PubSub, + + /// The command may have different outputs even starting from the same input arguments and key values. + /// Starting from Redis 7.0 this flag has been deprecated. Declaring a command as "random" can be done using + /// command tips, see https://redis.io/topics/command-tips. + Random, + + /// The command is allowed to run on slaves that don't serve stale data. Don't use if you don't know what + /// this means. + AllowStale, + + /// Don't propagate the command on monitor. Use this if the command has sensitive data among the arguments. + NoMonitor, + + /// Don't log this command in the slowlog. Use this if the command has sensitive data among the arguments. + NoSlowlog, + + /// The command time complexity is not greater than O(log(N)) where N is the size of the collection or + /// anything else representing the normal scalability issue with the command. + Fast, + + /// The command implements the interface to return the arguments that are keys. Used when start/stop/step + /// is not enough because of the command syntax. + GetkeysApi, + + /// The command should not register in Redis Cluster since is not designed to work with it because, for + /// example, is unable to report the position of the keys, programmatically creates key names, or any + /// other reason. + NoCluster, + + /// This command can be run by an un-authenticated client. Normally this is used by a command that is used + /// to authenticate a client. + NoAuth, + + /// This command may generate replication traffic, even though it's not a write command. + MayReplicate, + + /// All the keys this command may take are optional + NoMandatoryKeys, + + /// The command has the potential to block the client. + Blocking, + + /// Permit the command while the server is blocked either by a script or by a slow module command, see + /// RM_Yield. + AllowBusy, + + /// The command implements the interface to return the arguments that are channels. + GetchannelsApi, +} + +impl From<&RedisCommandFlags> for &'static str { + fn from(value: &RedisCommandFlags) -> Self { + match value { + RedisCommandFlags::Write => "write", + RedisCommandFlags::ReadOnly => "readonly", + RedisCommandFlags::Admin => "admin", + RedisCommandFlags::DenyOOM => "deny-oom", + RedisCommandFlags::DenyScript => "deny-script", + RedisCommandFlags::AllowLoading => "allow-loading", + RedisCommandFlags::PubSub => "pubsub", + RedisCommandFlags::Random => "random", + RedisCommandFlags::AllowStale => "allow-stale", + RedisCommandFlags::NoMonitor => "no-monitor", + RedisCommandFlags::NoSlowlog => "no-slowlog", + RedisCommandFlags::Fast => "fast", + RedisCommandFlags::GetkeysApi => "getkeys-api", + RedisCommandFlags::NoCluster => "no-cluster", + RedisCommandFlags::NoAuth => "no-auth", + RedisCommandFlags::MayReplicate => "may-replicate", + RedisCommandFlags::NoMandatoryKeys => "no-mandatory-keys", + RedisCommandFlags::Blocking => "blocking", + RedisCommandFlags::AllowBusy => "allow-busy", + RedisCommandFlags::GetchannelsApi => "getchannels-api", + } + } +} + #[derive(Debug, Deserialize)] pub enum RedisCommandKeySpecFlags { /// Read-Only. Reads the value of the key, but doesn't necessarily return it. @@ -111,7 +211,7 @@ pub struct KeySpecArg { #[derive(Debug, Deserialize)] struct Args { name: Option, - flags: Option, + flags: Vec, summary: Option, complexity: Option, since: Option, @@ -153,7 +253,15 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream let name_literal = args .name .unwrap_or_else(|| original_function_name.to_string()); - let flags_literal = to_token_stream(args.flags); + let flags_str = args + .flags + .into_iter() + .fold(String::new(), |s, v| { + format!("{} {}", s, Into::<&'static str>::into(&v)) + }) + .trim() + .to_owned(); + let flags_literal = quote!(#flags_str); let summary_literal = to_token_stream(args.summary); let complexity_literal = to_token_stream(args.complexity); let since_literal = to_token_stream(args.since); @@ -253,7 +361,7 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream ]; Ok(redis_module::commands::CommandInfo::new( #name_literal.to_owned(), - #flags_literal, + Some(#flags_literal.to_owned()), #summary_literal, #complexity_literal, #since_literal, From e2333781cb350fcdfbdd7a95fd59174fd04c47a1 Mon Sep 17 00:00:00 2001 From: meir Date: Mon, 15 May 2023 14:01:50 +0300 Subject: [PATCH 07/10] Review fixes --- examples/proc_macro_commands.rs | 1 - redismodule-rs-macros/src/lib.rs | 4 ++-- src/context/commands.rs | 24 ++++++++++++------------ 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/examples/proc_macro_commands.rs b/examples/proc_macro_commands.rs index 0f2b7d0e..50e86902 100644 --- a/examples/proc_macro_commands.rs +++ b/examples/proc_macro_commands.rs @@ -3,7 +3,6 @@ use redis_module_macros::command; #[command( { - name: "classic_keys", flags: [ReadOnly], arity: -2, key_spec: [ diff --git a/redismodule-rs-macros/src/lib.rs b/redismodule-rs-macros/src/lib.rs index b932e587..d947426d 100644 --- a/redismodule-rs-macros/src/lib.rs +++ b/redismodule-rs-macros/src/lib.rs @@ -7,7 +7,7 @@ mod command; /// This proc macro allow to specify that the follow function is a Redis command. /// The macro accept the following arguments that discribe the command properties: /// * name (optional) - The command name. in case not given, the function name will be taken. -/// * flags (optional) - Command flags such as `readonly`, for the full list please refer to https://redis.io/docs/reference/modules/modules-api-ref/#redismodule_createcommand +/// * flags - An array of `RedisCommandFlags`. /// * summary (optional) - Command summary /// * complexity (optional) - Command compexity /// * since (optional) - At which module version the command was first introduce @@ -60,7 +60,7 @@ mod command; /// #[command( /// { /// name: "test", -/// flags: "readonly", +/// flags: [ReadOnly], /// arity: -2, /// key_spec: [ /// { diff --git a/src/context/commands.rs b/src/context/commands.rs index 11aba730..3731540f 100644 --- a/src/context/commands.rs +++ b/src/context/commands.rs @@ -73,18 +73,18 @@ bitflags! { impl TryFrom<&str> for KeySpecFlags { type Error = RedisError; fn try_from(value: &str) -> Result { - match value { - "READ_ONLY" => Ok(KeySpecFlags::READ_ONLY), - "READ_WRITE" => Ok(KeySpecFlags::READ_WRITE), - "OVERWRITE" => Ok(KeySpecFlags::OVERWRITE), - "REMOVE" => Ok(KeySpecFlags::REMOVE), - "ACCESS" => Ok(KeySpecFlags::ACCESS), - "UPDATE" => Ok(KeySpecFlags::UPDATE), - "INSERT" => Ok(KeySpecFlags::INSERT), - "DELETE" => Ok(KeySpecFlags::DELETE), - "NOT_KEY" => Ok(KeySpecFlags::NOT_KEY), - "INCOMPLETE" => Ok(KeySpecFlags::INCOMPLETE), - "VARIABLE_FLAGS" => Ok(KeySpecFlags::VARIABLE_FLAGS), + match value.to_lowercase().as_str() { + "read_only" => Ok(KeySpecFlags::READ_ONLY), + "read_write" => Ok(KeySpecFlags::READ_WRITE), + "overwrite" => Ok(KeySpecFlags::OVERWRITE), + "remove" => Ok(KeySpecFlags::REMOVE), + "access" => Ok(KeySpecFlags::ACCESS), + "update" => Ok(KeySpecFlags::UPDATE), + "insert" => Ok(KeySpecFlags::INSERT), + "delete" => Ok(KeySpecFlags::DELETE), + "not_key" => Ok(KeySpecFlags::NOT_KEY), + "incomplete" => Ok(KeySpecFlags::INCOMPLETE), + "variable_flags" => Ok(KeySpecFlags::VARIABLE_FLAGS), _ => Err(RedisError::String(format!( "Value {value} is not a valid key spec flag." ))), From d1f3e005ffe65a43218e3623e497f8d712df599a Mon Sep 17 00:00:00 2001 From: meir Date: Mon, 15 May 2023 14:32:19 +0300 Subject: [PATCH 08/10] Review fixes --- src/context/commands.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/context/commands.rs b/src/context/commands.rs index 3731540f..dd2737ef 100644 --- a/src/context/commands.rs +++ b/src/context/commands.rs @@ -230,7 +230,7 @@ impl From<&KeySpec> for raw::RedisModuleCommandKeySpec { } } -type CommnadCallback = +type CommandCallback = extern "C" fn(*mut raw::RedisModuleCtx, *mut *mut raw::RedisModuleString, i32) -> i32; /// A struct represent a CommandInfo @@ -243,7 +243,7 @@ pub struct CommandInfo { tips: Option, arity: i64, key_spec: Vec, - callback: CommnadCallback, + callback: CommandCallback, } impl CommandInfo { @@ -256,7 +256,7 @@ impl CommandInfo { tips: Option, arity: i64, key_spec: Vec, - callback: CommnadCallback, + callback: CommandCallback, ) -> CommandInfo { CommandInfo { name, From 5356990fb946c662ae15cf7bdd64bc0441822bd6 Mon Sep 17 00:00:00 2001 From: meir Date: Mon, 15 May 2023 15:07:44 +0300 Subject: [PATCH 09/10] Free command info structure --- src/context/commands.rs | 54 ++++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/src/context/commands.rs b/src/context/commands.rs index dd2737ef..bebc45a4 100644 --- a/src/context/commands.rs +++ b/src/context/commands.rs @@ -3,6 +3,7 @@ use crate::Context; use crate::RedisError; use crate::Status; use bitflags::bitflags; +use libc::c_char; use linkme::distributed_slice; use redis_module_macros_internals::api; use std::ffi::CString; @@ -275,14 +276,12 @@ impl CommandInfo { #[distributed_slice()] pub static COMMNADS_LIST: [fn() -> Result] = [..]; -pub fn get_redis_key_spec(key_spec: Vec) -> *mut raw::RedisModuleCommandKeySpec { +pub fn get_redis_key_spec(key_spec: Vec) -> Vec { let mut redis_key_spec: Vec = key_spec.into_iter().map(|v| (&v).into()).collect(); let zerod: raw::RedisModuleCommandKeySpec = unsafe { MaybeUninit::zeroed().assume_init() }; redis_key_spec.push(zerod); - let res = redis_key_spec.as_ptr(); - std::mem::forget(redis_key_spec); - res as *mut raw::RedisModuleCommandKeySpec + redis_key_spec } api! {[ @@ -335,43 +334,58 @@ api! {[ let summary = command_info .summary .as_ref() - .map(|v| CString::new(v.as_str()).unwrap().into_raw()) - .unwrap_or(ptr::null_mut()); + .map(|v| Some(CString::new(v.as_str()).unwrap())) + .unwrap_or(None); let complexity = command_info .complexity .as_ref() - .map(|v| CString::new(v.as_str()).unwrap().into_raw()) - .unwrap_or(ptr::null_mut()); + .map(|v| Some(CString::new(v.as_str()).unwrap())) + .unwrap_or(None); let since = command_info .since .as_ref() - .map(|v| CString::new(v.as_str()).unwrap().into_raw()) - .unwrap_or(ptr::null_mut()); + .map(|v| Some(CString::new(v.as_str()).unwrap())) + .unwrap_or(None); let tips = command_info .tips .as_ref() - .map(|v| CString::new(v.as_str()).unwrap().into_raw()) - .unwrap_or(ptr::null_mut()); + .map(|v| Some(CString::new(v.as_str()).unwrap())) + .unwrap_or(None); + + let key_specs = get_redis_key_spec(command_info.key_spec); - let redis_command_info = Box::into_raw(Box::new(raw::RedisModuleCommandInfo { + let mut redis_command_info = raw::RedisModuleCommandInfo { version: &COMMNAD_INFO_VERSION, - summary, - complexity, - since, + summary: summary.as_ref().map(|v| v.as_ptr()).unwrap_or(ptr::null_mut()), + complexity: complexity.as_ref().map(|v| v.as_ptr()).unwrap_or(ptr::null_mut()), + since: since.as_ref().map(|v| v.as_ptr()).unwrap_or(ptr::null_mut()), history: ptr::null_mut(), // currently we will not support history - tips, + tips: tips.as_ref().map(|v| v.as_ptr()).unwrap_or(ptr::null_mut()), arity: command_info.arity as c_int, - key_specs: get_redis_key_spec(command_info.key_spec), + key_specs: key_specs.as_ptr() as *mut raw::RedisModuleCommandKeySpec, args: ptr::null_mut(), - })); + }; - if unsafe { RedisModule_SetCommandInfo(command, redis_command_info) } == raw::Status::Err as i32 { + if unsafe { RedisModule_SetCommandInfo(command, &mut redis_command_info as *mut raw::RedisModuleCommandInfo) } == raw::Status::Err as i32 { return Err(RedisError::String(format!( "Failed setting info for command {}.", command_info.name ))); } + // the only CString pointers which are not freed are those of the key_specs, lets free them here. + key_specs.into_iter().for_each(|v|{ + if !v.notes.is_null() { + unsafe{CString::from_raw(v.notes as *mut c_char)}; + } + if v.begin_search_type == raw::RedisModuleKeySpecBeginSearchType_REDISMODULE_KSPEC_BS_KEYWORD { + let keyword = unsafe{v.bs.keyword.keyword}; + if !keyword.is_null() { + unsafe{CString::from_raw(v.bs.keyword.keyword as *mut c_char)}; + } + } + }); + Ok(()) }) } From c8e55e012c2747375a972a343a93fa667d519cf9 Mon Sep 17 00:00:00 2001 From: meir Date: Mon, 15 May 2023 16:45:26 +0300 Subject: [PATCH 10/10] Review fixes --- redismodule-rs-macros/src/command.rs | 8 +- src/context/commands.rs | 117 ++++++++++++++++++++++----- 2 files changed, 101 insertions(+), 24 deletions(-) diff --git a/redismodule-rs-macros/src/command.rs b/redismodule-rs-macros/src/command.rs index 4f6ab064..f6e8584e 100644 --- a/redismodule-rs-macros/src/command.rs +++ b/redismodule-rs-macros/src/command.rs @@ -296,14 +296,14 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream BeginSearch::Index(i) => { let i = i.index; quote! { - redis_module::commands::BeginSearch::Index(#i) + redis_module::commands::BeginSearch::new_index(#i) } } BeginSearch::Keyword(begin_search_keyword) => { let k = begin_search_keyword.keyword.as_str(); let i = begin_search_keyword.startfrom; quote! { - redis_module::commands::BeginSearch::Keyword((#k.to_owned(), #i)) + redis_module::commands::BeginSearch::new_keyword(#k.to_owned(), #i) } } }) @@ -318,7 +318,7 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream let firstkey = find_keys_num.first_key; let keystep = find_keys_num.key_step; quote! { - redis_module::commands::FindKeys::Keynum((#keynumidx, #firstkey, #keystep)) + redis_module::commands::FindKeys::new_keys_num(#keynumidx, #firstkey, #keystep) } } FindKeys::Range(find_keys_range) => { @@ -326,7 +326,7 @@ pub(crate) fn redis_command(attr: TokenStream, item: TokenStream) -> TokenStream let steps = find_keys_range.steps; let limit = find_keys_range.limit; quote! { - redis_module::commands::FindKeys::Range((#last_key, #steps, #limit)) + redis_module::commands::FindKeys::new_range(#last_key, #steps, #limit) } } }) diff --git a/src/context/commands.rs b/src/context/commands.rs index bebc45a4..2795ffae 100644 --- a/src/context/commands.rs +++ b/src/context/commands.rs @@ -101,13 +101,38 @@ impl From> for KeySpecFlags { } } +/// A version of begin search spec that finds the index +/// indicating where to start search for keys based on +/// an index. +pub struct BeginSearchIndex { + index: i32, +} + +/// A version of begin search spec that finds the index +/// indicating where to start search for keys based on +/// a keyword. +pub struct BeginSearchKeyword { + keyword: String, + startfrom: i32, +} + /// This struct represents how Redis should start looking for keys. /// There are 2 possible options: /// 1. Index - start looking for keys from a given position. /// 2. Keyword - Search for a specific keyward and start looking for keys from this keyword pub enum BeginSearch { - Index(i32), - Keyword((String, i32)), // (keyword, startfrom) + Index(BeginSearchIndex), + Keyword(BeginSearchKeyword), +} + +impl BeginSearch { + pub fn new_index(index: i32) -> BeginSearch { + BeginSearch::Index(BeginSearchIndex { index }) + } + + pub fn new_keyword(keyword: String, startfrom: i32) -> BeginSearch { + BeginSearch::Keyword(BeginSearchKeyword { keyword, startfrom }) + } } impl From<&BeginSearch> @@ -118,20 +143,24 @@ impl From<&BeginSearch> { fn from(value: &BeginSearch) -> Self { match value { - BeginSearch::Index(i) => ( + BeginSearch::Index(index_spec) => ( raw::RedisModuleKeySpecBeginSearchType_REDISMODULE_KSPEC_BS_INDEX, raw::RedisModuleCommandKeySpec__bindgen_ty_1 { - index: raw::RedisModuleCommandKeySpec__bindgen_ty_1__bindgen_ty_1 { pos: *i }, + index: raw::RedisModuleCommandKeySpec__bindgen_ty_1__bindgen_ty_1 { + pos: index_spec.index, + }, }, ), - BeginSearch::Keyword((k, i)) => { - let keyword = CString::new(k.as_str()).unwrap().into_raw(); + BeginSearch::Keyword(keyword_spec) => { + let keyword = CString::new(keyword_spec.keyword.as_str()) + .unwrap() + .into_raw(); ( raw::RedisModuleKeySpecBeginSearchType_REDISMODULE_KSPEC_BS_KEYWORD, raw::RedisModuleCommandKeySpec__bindgen_ty_1 { keyword: raw::RedisModuleCommandKeySpec__bindgen_ty_1__bindgen_ty_2 { keyword, - startfrom: *i, + startfrom: keyword_spec.startfrom, }, }, ) @@ -140,17 +169,65 @@ impl From<&BeginSearch> } } +/// A version of find keys base on range. +/// * `last_key` - Index of the last key relative to the result of the +/// begin search step. Can be negative, in which case it's not +/// relative. -1 indicates the last argument, -2 one before the +/// last and so on. +/// * `steps` - How many arguments should we skip after finding a +/// key, in order to find the next one. +/// * `limit` - If `lastkey` is -1, we use `limit` to stop the search +/// by a factor. 0 and 1 mean no limit. 2 means 1/2 of the +/// remaining args, 3 means 1/3, and so on. +pub struct FindKeysRange { + last_key: i32, + steps: i32, + limit: i32, +} + +/// A version of find keys base on some argument representing the number of keys +/// * keynumidx - Index of the argument containing the number of +/// keys to come, relative to the result of the begin search step. +/// * firstkey - Index of the fist key relative to the result of the +/// begin search step. (Usually it's just after `keynumidx`, in +/// which case it should be set to `keynumidx + 1`.) +/// * keystep - How many arguments should we skip after finding a +/// key, in order to find the next one? +pub struct FindKeysNum { + key_num_idx: i32, + first_key: i32, + key_step: i32, +} + /// After Redis finds the location from where it needs to start looking for keys, -/// Redis will start finding keys base on the information in this struct. +/// Redis will start finding keys base on the information in this enum. /// There are 2 possible options: -/// 1. Range - A tuple represent a range of `(last_key, steps, limit)`. -/// 2. Keynum - A tuple of 3 elements `(keynumidx, firstkey, keystep)`. +/// 1. Range - Required to specify additional 3 more values, `last_key`, `steps`, and `limit`. +/// 2. Keynum - Required to specify additional 3 more values, `keynumidx`, `firstkey`, and `keystep`. /// Redis will consider the argument at `keynumidx` as an indicator /// to the number of keys that will follow. Then it will start /// from `firstkey` and jump each `keystep` to find the keys. pub enum FindKeys { - Range((i32, i32, i32)), // (last_key, steps, limit) - Keynum((i32, i32, i32)), // (keynumidx, firstkey, keystep) + Range(FindKeysRange), + Keynum(FindKeysNum), +} + +impl FindKeys { + pub fn new_range(last_key: i32, steps: i32, limit: i32) -> FindKeys { + FindKeys::Range(FindKeysRange { + last_key, + steps, + limit, + }) + } + + pub fn new_keys_num(key_num_idx: i32, first_key: i32, key_step: i32) -> FindKeys { + FindKeys::Keynum(FindKeysNum { + key_num_idx, + first_key, + key_step, + }) + } } impl From<&FindKeys> @@ -161,23 +238,23 @@ impl From<&FindKeys> { fn from(value: &FindKeys) -> Self { match value { - FindKeys::Range((lastkey, keystep, limit)) => ( + FindKeys::Range(range_spec) => ( raw::RedisModuleKeySpecFindKeysType_REDISMODULE_KSPEC_FK_RANGE, raw::RedisModuleCommandKeySpec__bindgen_ty_2 { range: raw::RedisModuleCommandKeySpec__bindgen_ty_2__bindgen_ty_1 { - lastkey: *lastkey, - keystep: *keystep, - limit: *limit, + lastkey: range_spec.last_key, + keystep: range_spec.steps, + limit: range_spec.limit, }, }, ), - FindKeys::Keynum((keynumidx, firstkey, keystep)) => ( + FindKeys::Keynum(keynum_spec) => ( raw::RedisModuleKeySpecFindKeysType_REDISMODULE_KSPEC_FK_KEYNUM, raw::RedisModuleCommandKeySpec__bindgen_ty_2 { keynum: raw::RedisModuleCommandKeySpec__bindgen_ty_2__bindgen_ty_2 { - keynumidx: *keynumidx, - firstkey: *firstkey, - keystep: *keystep, + keynumidx: keynum_spec.key_num_idx, + firstkey: keynum_spec.first_key, + keystep: keynum_spec.key_step, }, }, ),