From 2de7eccd5f419186169917be07f2c9fae8109954 Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Tue, 26 Aug 2025 16:13:20 +0200 Subject: [PATCH 01/17] add fields to KeyMode bitflags --- Cargo.toml | 5 +++++ build.rs | 9 +++------ src/key.rs | 8 +++++--- src/raw.rs | 25 +++++++++++++++---------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b084c3cb..b1aba904 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -133,6 +133,7 @@ serde = { version = "1", features = ["derive"] } nix = { version = "0.26", default-features = false } cfg-if = "1" redis-module-macros-internals = { path = "./redismodule-rs-macros-internals" } +strum = {version = "0.27.2",features = ["derive"]} log = "0.4" [dev-dependencies] @@ -146,6 +147,10 @@ redis-module = { path = "./", default-features = false } bindgen = { version = "0.66", default-features = false, features = ["logging", "prettyplease"]} cc = "1" +#[build-dependencies.bindgen] +#version = "0.71.1" +#default-features = false + [features] default = ["min-redis-compatibility-version-6-0", "bindgen-runtime"] diff --git a/build.rs b/build.rs index 707d99ad..4b655094 100644 --- a/build.rs +++ b/build.rs @@ -12,12 +12,9 @@ impl ParseCallbacks for RedisModuleCallback { fn int_macro(&self, name: &str, _value: i64) -> Option { if name.starts_with("REDISMODULE_SUBEVENT_") || name.starts_with("REDISMODULE_EVENT_") { Some(IntKind::U64) - } else if name.starts_with("REDISMODULE_REPLY_") - || name.starts_with("REDISMODULE_KEYTYPE_") - || name.starts_with("REDISMODULE_AUX_") - || name == "REDISMODULE_OK" - || name == "REDISMODULE_ERR" - || name == "REDISMODULE_LIST_HEAD" + } else if name.starts_with("REDISMODULE_REPLY") { + Some(IntKind::I32) + } else if name == "REDISMODULE_LIST_HEAD" || name == "REDISMODULE_LIST_TAIL" { // These values are used as `enum` discriminants, and thus must be `isize`. diff --git a/src/key.rs b/src/key.rs index 9e363a11..76b7b966 100644 --- a/src/key.rs +++ b/src/key.rs @@ -110,7 +110,8 @@ impl RedisKey { /// Will panic if `RedisModule_KeyType` is missing in redismodule.h #[must_use] pub fn key_type(&self) -> raw::KeyType { - unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }.into() + let discriminant = unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }; + KeyType::from_repr(discriminant as u32).unwrap() } /// Detects whether the key pointer given to us by Redis is null. @@ -379,7 +380,8 @@ impl RedisKeyWritable { /// Will panic if `RedisModule_KeyType` is missing in redismodule.h #[must_use] pub fn key_type(&self) -> raw::KeyType { - unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }.into() + let discriminant = unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }; + KeyType::from_repr(discriminant as u32).unwrap() } pub fn open_with_redis_string( @@ -670,7 +672,7 @@ fn to_raw_mode(mode: KeyMode) -> raw::KeyMode { /// Will panic if `RedisModule_KeyType` or `RedisModule_ModuleTypeGetType` are missing in redismodule.h #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn verify_type(key_inner: *mut raw::RedisModuleKey, redis_type: &RedisType) -> RedisResult { - let key_type: KeyType = unsafe { raw::RedisModule_KeyType.unwrap()(key_inner) }.into(); + let key_type: KeyType = KeyType::from_repr(unsafe { raw::RedisModule_KeyType.unwrap()(key_inner) as u32 }).unwrap(); if key_type != KeyType::Empty { // The key exists; check its type diff --git a/src/raw.rs b/src/raw.rs index 71622f3f..1e4b6cb5 100644 --- a/src/raw.rs +++ b/src/raw.rs @@ -29,6 +29,13 @@ bitflags! { pub struct KeyMode: c_int { const READ = REDISMODULE_READ as c_int; const WRITE = REDISMODULE_WRITE as c_int; + + const KEYOPEN_NOTOUCH = REDISMODULE_OPEN_KEY_NOTOUCH as c_int; + const KEYOPEN_NONOTIFY = REDISMODULE_OPEN_KEY_NONOTIFY as c_int; + const KEYOPEN_NOSTATS = REDISMODULE_OPEN_KEY_NOSTATS as c_int; + const KEYOPEN_NOEFFECTS = REDISMODULE_OPEN_KEY_NOEFFECTS as c_int; + const KEYOPEN_NOEXPIRE = REDISMODULE_OPEN_KEY_NOEXPIRE as c_int; + const KEYOPEN_ACCESSEXPIRED = REDISMODULE_OPEN_KEY_ACCESS_EXPIRED as c_int; } } @@ -40,7 +47,8 @@ bitflags! { } } -#[derive(Primitive, Debug, PartialEq, Eq)] +#[repr(u32)] +#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] pub enum KeyType { Empty = REDISMODULE_KEYTYPE_EMPTY, String = REDISMODULE_KEYTYPE_STRING, @@ -52,19 +60,14 @@ pub enum KeyType { Stream = REDISMODULE_KEYTYPE_STREAM, } -impl From for KeyType { - fn from(v: c_int) -> Self { - Self::from_i32(v).unwrap() - } -} - #[derive(Primitive, Debug, PartialEq, Eq)] pub enum Where { ListHead = REDISMODULE_LIST_HEAD, ListTail = REDISMODULE_LIST_TAIL, } -#[derive(Primitive, Debug, PartialEq, Eq)] +#[repr(i32)] +#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] pub enum ReplyType { Unknown = REDISMODULE_REPLY_UNKNOWN, String = REDISMODULE_REPLY_STRING, @@ -86,13 +89,15 @@ impl From for ReplyType { } } -#[derive(Primitive, Debug, PartialEq, Eq)] +#[repr(u32)] +#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] pub enum Aux { Before = REDISMODULE_AUX_BEFORE_RDB, After = REDISMODULE_AUX_AFTER_RDB, } -#[derive(Primitive, Debug, PartialEq, Eq)] +#[repr(u32)] +#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] pub enum Status { Ok = REDISMODULE_OK, Err = REDISMODULE_ERR, From 42976c7ed53132dec99c46f55ea97ee6d20d6ca2 Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Tue, 26 Aug 2025 18:19:58 +0200 Subject: [PATCH 02/17] add from_raw_parts creation method to RedisString --- src/redismodule.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/redismodule.rs b/src/redismodule.rs index 46ed3ec1..adba5c0f 100644 --- a/src/redismodule.rs +++ b/src/redismodule.rs @@ -161,6 +161,22 @@ impl RedisString { Self { ctx, inner } } + /// Create a RedisString from a raw C string and length. The String will be copied. + /// + /// # Safety + /// The caller must ensure that the provided pointer is valid and points to a memory region + /// that is at least `len` bytes long. + #[allow(clippy::not_unsafe_ptr_arg_deref)] + pub unsafe fn from_raw_parts(ctx: Option>, s: *const c_char, len: libc::size_t) -> Self { + let ctx = ctx.map_or(std::ptr::null_mut(), |v| v.as_ptr()); + + let inner = unsafe { + raw::RedisModule_CreateString.unwrap()(ctx, s, len) + }; + + Self { ctx, inner } + } + #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn create_from_slice(ctx: *mut raw::RedisModuleCtx, s: &[u8]) -> Self { let inner = unsafe { @@ -201,6 +217,13 @@ impl RedisString { len == 0 } + #[must_use] + pub fn as_cstr_ptr_and_len(&self) -> (*const c_char, usize) { + let mut len: usize = 0; + let ptr = raw::string_ptr_len(self.inner, &mut len); + (ptr, len) + } + pub fn try_as_str<'a>(&self) -> Result<&'a str, RedisError> { Self::from_ptr(self.inner).map_err(|_| RedisError::Str("Couldn't parse as UTF-8 string")) } From 5f09b81b10d08d93b48a535a7dba25c998a56f9e Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Wed, 27 Aug 2025 15:12:13 +0200 Subject: [PATCH 03/17] implmement RedisModule_ScanKey as foreach function with closure and Rust iterator using a KeyScanCursor type add example todo integration tests --- examples/scan_keys.rs | 108 +++++++++++++- src/context/key_scan_cursor.rs | 247 +++++++++++++++++++++++++++++++++ src/context/mod.rs | 1 + src/lib.rs | 1 + src/redismodule.rs | 4 + 5 files changed, 360 insertions(+), 1 deletion(-) create mode 100644 src/context/key_scan_cursor.rs diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index f1d84911..c8e11521 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -1,7 +1,58 @@ +use std::cell::RefCell; + +// This example shows the usage of the scan functionality of the Rust Redis Module API Wrapper. +// +// The example implements three commands: +// +// 1. `scan_keys` - scans all keys in the database and returns their names as an array of RedisString. +// 2. `scan_key_it ` - scans all fields and values in a hash key providing an iterator and return the field/value pairs as an array of RedisString. +// 3. `scan_key_fe ` - scans all fields and values in a hash key using a closure that stores tthe field/value pairs as an array of RedisString. +// +// `scan_key_it` always copies the field and value strings, while `scan_key_fe` uses references to the field and value strings. In that example +// both implementations need to clone the strings, because we want to return them as an array of RedisString. +// +// ## Usage examples for scan_key: +// +// First we need to setup a hash key with some fields and values: +// +// ``` +// HSET user:123 name Alice age 29 location Austin +// ``` +// +// We need to start redis-server with the module loaded (example for macOS, adjust path as needed): +// +// ``` +// redis-server --loadmodule ./target/debug/examples/libscan_keys.dylib +// ``` +// +// Afterwards we can call the commands via redis-cli: +// +// ``` +// > SCAN_KEYS +// 1) "user:123" +// +// > SCAN_KEY_IT user:123 +// 1) "name" +// 2) "Alice" +// 3) "age" +// 4) "29" +// 5) "location" +// 6) "Austin" +// +// > SCAN_KEY_FE user:123 +// 1) "name" +// 2) "Alice" +// 3) "age" +// 4) "29" +// 5) "location" +// 6) "Austin" +// ``` + use redis_module::{ - key::RedisKey, redis_module, Context, KeysCursor, RedisResult, RedisString, RedisValue, + key::{KeyFlags, RedisKey}, redis_module, Context, KeysCursor, RedisError, RedisResult, RedisString, RedisValue, ScanKeyCursor }; +/// Scans all keys in the database and returns their names as an array of RedisString. fn scan_keys(ctx: &Context, _args: Vec) -> RedisResult { let cursor = KeysCursor::new(); let mut res = Vec::new(); @@ -16,6 +67,59 @@ fn scan_keys(ctx: &Context, _args: Vec) -> RedisResult { Ok(RedisValue::Array(res)) } +/// Scans all fields and values in a hash key and returns them as an array of RedisString. +/// The command takes one argument: the name of the hash key to scan. +fn scan_key_foreach(ctx: &Context, args: Vec) -> RedisResult { + // only argument is the key name + if args.len() != 2 { + return Err(RedisError::WrongArity); + } + + let key_name = &args[1]; + let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); + let ty = key.key_type(); + let cursor = ScanKeyCursor::new(key); + let out = format!("Cursor created for Scanning key: {}, type={:?}", key_name, ty); + ctx.log_notice(&out); + + let res = RefCell::new(Vec::new()); + cursor.foreach(|_key, field, value| { + let out = format!("Field: {}, Value: {}", field, value); + ctx.log_notice(&out); + + let mut res = res.borrow_mut(); + res.push(RedisValue::BulkRedisString(field.clone())); + res.push(RedisValue::BulkRedisString(value.clone())); + }); + + Ok(RedisValue::Array(res.take())) +} + +/// Scans all fields and values in a hash key and returns them as an array of RedisString. +/// The command takes one argument: the name of the hash key to scan. +fn scan_key_iterator(ctx: &Context, args: Vec) -> RedisResult { + // only argument is the key name + if args.len() != 2 { + return Err(RedisError::WrongArity); + } + + let key_name = &args[1]; + let mut res = Vec::new(); + let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); + let ty = key.key_type(); + let cursor = ScanKeyCursor::new(key); + let out = format!("Cursor created for Scanning key: {}, type={:?}", key_name, ty); + ctx.log_notice(&out); + + for (no, (field, value)) in cursor.iter().enumerate() { + let out = format!("It: {}, Field: {}, Value: {}", no, field, value); + ctx.log_notice(&out); + res.push(RedisValue::BulkRedisString(field)); + res.push(RedisValue::BulkRedisString(value)); + } + Ok(RedisValue::Array(res)) +} + ////////////////////////////////////////////////////// redis_module! { @@ -25,5 +129,7 @@ redis_module! { data_types: [], commands: [ ["scan_keys", scan_keys, "readonly", 0, 0, 0, ""], + ["scan_key_it", scan_key_iterator, "readonly", 0, 0, 0, ""], + ["scan_key_fe", scan_key_foreach, "readonly", 0, 0, 0, ""], ], } diff --git a/src/context/key_scan_cursor.rs b/src/context/key_scan_cursor.rs new file mode 100644 index 00000000..68ebb095 --- /dev/null +++ b/src/context/key_scan_cursor.rs @@ -0,0 +1,247 @@ +use std::{ffi::c_void, ptr::{self, addr_of_mut}}; + +use crate::{key::RedisKey, raw, Context, RedisString}; + +/// A cursor to scan fields and values in a hash key. +/// +/// This is a wrapper around the RedisModule_ScanKey function from the C API. It provides access via [`ScanKeyCursor::foreach] and provides +/// a Rust iterator. +/// +/// Example usage: +/// ```no_run +/// +/// ``` +/// +/// The iterator yields tuples of (field: RedisString, value: RedisString). +/// +/// ## Implementation notes +/// +/// The `RedisModule_ScanKey` function from the C API uses a callback to return the field and value strings. We +/// distinguish two cases: +/// +/// 1. Either the callback is called once, +/// 2. or multiple times +/// +/// and this depends if a rehash happens during the scan. +pub struct ScanKeyCursor { + key: RedisKey, + inner_cursor: *mut raw::RedisModuleScanCursor, +} + +//type ScanKeyCallback = F where F: FnMut(&RedisKey, &RedisString, &RedisString); + +impl ScanKeyCursor { + pub fn new(key: RedisKey) -> Self { + let inner_cursor = unsafe { raw::RedisModule_ScanCursorCreate.unwrap()() }; + Self { key, inner_cursor } + } + + pub fn restart(&self) { + unsafe { raw::RedisModule_ScanCursorRestart.unwrap()(self.inner_cursor) }; + } + + /// Implements a callback based foreach loop over all fields and values in the hash key, use for optimal performance. + pub fn foreach(&self, f: F) { + // Safety: Assumption: c-side initialized the function ptr and it is is never changed, + // i.e. after module initialization the function pointers stay valid till the end of the program. + let scan_key = unsafe { raw::RedisModule_ScanKey.unwrap() }; + + let mut res = 1; + while res != 0 { + res = unsafe { + scan_key( + self.key.key_inner, + self.inner_cursor, + Some(foreach_callback::), + &f as *const F as *mut c_void, + ) + } + } + } + + pub fn iter(&self) -> ScanKeyCursorIterator<'_> { + let ctx = Context::new(self.key.ctx); + ctx.log_notice("Starting ScanKeyCursor iteration"); + ScanKeyCursorIterator { + cursor: self, + buf: Vec::new(), + last_call: false, + } + } +} + +impl Drop for ScanKeyCursor { + fn drop(&mut self) { + unsafe { raw::RedisModule_ScanCursorDestroy.unwrap()(self.inner_cursor) }; + } +} + +pub type ScanKeyIteratorItem = (RedisString, RedisString); + +pub struct ScanKeyCursorIterator<'a> { + /// The cursor that is used for the iteration + cursor: &'a ScanKeyCursor, + + /// Buffer to hold the uninitialized data if the C callback is called multiple times. + buf: Vec, + + /// Stores a flag that indicates if scan_key needs to be called again + last_call: bool, +} + +enum IteratorState { + NeedToCallScanKey, + HasBufferedItems, + Done, +} + +enum StackSlotState { + Empty, + Filled(ScanKeyIteratorItem), +} + +struct StackSlot<'a> { + state: StackSlotState, + ctx: Context, + buf: &'a mut Vec, +} + +impl ScanKeyCursorIterator<'_> { + fn current_state(&self) -> IteratorState { + if !self.buf.is_empty() { + IteratorState::HasBufferedItems + } else if self.last_call { + IteratorState::Done + } else { + IteratorState::NeedToCallScanKey + } + } + + fn next_scan_call(&mut self) -> Option { + let ctx_ptr = self.cursor.key.ctx; + let ctx = Context::new(ctx_ptr); + + let mut stack_slot = StackSlot { + state: StackSlotState::Empty, + ctx: Context::new(ctx_ptr), + buf: &mut self.buf + }; + + let data_ptr = addr_of_mut!(stack_slot).cast::(); + + // Safety: Assumption: c-side initialized the function ptr and it is is never changed, + // i.e. after module initialization the function pointers stay valid till the end of the program. + let scan_key = unsafe { raw::RedisModule_ScanKey.unwrap() }; + + // Safety: All pointers we pass here are guaranteed to remain valid during the `scan_key` call. + let ret = unsafe { + scan_key( + self.cursor.key.key_inner, + self.cursor.inner_cursor, + Some(iterator_callback), + data_ptr, + ) + }; + + // Check if more calls are needed + if ret == 0 { + self.last_call = true; + // we may still have buffered items + } + + let StackSlotState::Filled(reval) = stack_slot.state else { + // should not happen + panic!("ScanKey callback did not fill the stack slot"); + }; + + ctx.log_notice(&format!("next Reval field: {}, value: {}", reval.0, reval.1)); + + Some(reval) + } + + fn next_buffered_item(&mut self) -> Option { + // todo: use different datatype / access pattern for performance + Some(self.buf.remove(0)) + } +} + +/// The callback that is called by `RedisModule_ScanKey` to return the field and value strings. +/// +/// The `data` pointer is a stack slot of type `RawData` that is used to pass the data back to the iterator. +unsafe extern "C" fn foreach_callback( + key: *mut raw::RedisModuleKey, + field: *mut raw::RedisModuleString, + value: *mut raw::RedisModuleString, + data: *mut c_void, +) { + let ctx = ptr::null_mut(); + let key = RedisKey::from_raw_parts(ctx, key); + + let field = RedisString::from_redis_module_string(ctx, field); + let value = RedisString::from_redis_module_string(ctx, value); + + let callback = unsafe { &mut *(data.cast::()) }; + callback(&key, &field, &value); + + // we're not the owner of field and value strings + field.take(); + value.take(); + + key.take(); // we're not the owner of the key either +} + +/// The callback that is called by `RedisModule_ScanKey` to return the field and value strings. +/// +/// The `data` pointer is a stack slot of type `RawData` that is used to pass the data back to the iterator. +unsafe extern "C" fn iterator_callback( + _key: *mut raw::RedisModuleKey, + field: *mut raw::RedisModuleString, + value: *mut raw::RedisModuleString, + data: *mut c_void, +) { + // SAFETY: this is the responsibility of the caller, see only usage below in `next()` + // `data` is a stack slot of type Data + let slot = data.cast::(); + let slot = &mut (*slot); + + // todo: use new-type with refcount handling for better performance + let field = raw::RedisModule_CreateStringFromString.unwrap()(slot.ctx.get_raw(), field); + let value = raw::RedisModule_CreateStringFromString.unwrap()(slot.ctx.get_raw(), value); + + let field = RedisString::from_redis_module_string(slot.ctx.get_raw(), field); + let value = RedisString::from_redis_module_string(slot.ctx.get_raw(), value); + + match slot.state { + StackSlotState::Empty => { + let out = format!("CB - Fill empty slot - Field: {}, Value: {}", field, value); + slot.ctx.log_notice(&out); + slot.state = StackSlotState::Filled((field, value)); + } + StackSlotState::Filled(_) => { + // This is the case where the callback is called multiple times. + // We need to buffer the data in the iterator state. + let out = format!("CB - Buffer for future use - Field: {}, Value: {}", field, value); + slot.ctx.log_notice(&out); + slot.buf.push((field, value)); + + } + } + +} + +// Implements an iterator for `KeysCursor` that yields (RedisKey, *mut RedisModuleString, *mut RedisModuleString) in a Rust for loop. +// This is a wrapper around the RedisModule_ScanKey function from the C API and uses a pattern to get the values from the callback that +// is also used in stack unwinding scenarios. There is not common term for that but here we can think of it as a "stack slot" pattern. +impl Iterator for ScanKeyCursorIterator<'_> { + type Item = ScanKeyIteratorItem; + + fn next(&mut self) -> Option { + let ctx = Context::new(self.cursor.key.ctx); + ctx.log_notice("ScanKeyCursorIterator next() called"); + match self.current_state() { + IteratorState::NeedToCallScanKey => self.next_scan_call(), + IteratorState::HasBufferedItems => self.next_buffered_item(), + IteratorState::Done => None, + } + } +} \ No newline at end of file diff --git a/src/context/mod.rs b/src/context/mod.rs index 00371d3f..e2ebe629 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -32,6 +32,7 @@ pub mod call_reply; pub mod commands; pub mod defrag; pub mod info; +pub mod key_scan_cursor; pub mod keys_cursor; pub mod server_events; pub mod thread_safe; diff --git a/src/lib.rs b/src/lib.rs index b4195b78..4b8f28fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,6 +32,7 @@ pub use crate::context::call_reply::{CallReply, CallResult, ErrorReply, PromiseC pub use crate::context::commands; pub use crate::context::defrag; pub use crate::context::keys_cursor::KeysCursor; +pub use crate::context::key_scan_cursor::ScanKeyCursor; pub use crate::context::server_events; pub use crate::context::AclCategory; pub use crate::context::AclPermissions; diff --git a/src/redismodule.rs b/src/redismodule.rs index adba5c0f..3ba12b22 100644 --- a/src/redismodule.rs +++ b/src/redismodule.rs @@ -285,6 +285,10 @@ impl RedisString { impl Drop for RedisString { fn drop(&mut self) { if !self.inner.is_null() { + let ctx = Context::new(self.ctx); + let out = format!("Dropping RedisString '{}'", self.to_string_lossy()); + ctx.log_notice(&out); + unsafe { raw::RedisModule_FreeString.unwrap()(self.ctx, self.inner); } From 1c912080b4bb4252626f91b04c31d432307a2bc8 Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Thu, 28 Aug 2025 12:53:47 +0200 Subject: [PATCH 04/17] fix iterator crash with wrong keyname --- src/context/key_scan_cursor.rs | 90 +++++++++++++++------------------- 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/src/context/key_scan_cursor.rs b/src/context/key_scan_cursor.rs index 68ebb095..8ca45a5c 100644 --- a/src/context/key_scan_cursor.rs +++ b/src/context/key_scan_cursor.rs @@ -1,30 +1,33 @@ -use std::{ffi::c_void, ptr::{self, addr_of_mut}}; +use std::{ + ffi::c_void, + ptr::{self, addr_of_mut}, +}; use crate::{key::RedisKey, raw, Context, RedisString}; /// A cursor to scan fields and values in a hash key. -/// +/// /// This is a wrapper around the RedisModule_ScanKey function from the C API. It provides access via [`ScanKeyCursor::foreach] and provides /// a Rust iterator. -/// +/// /// Example usage: /// ```no_run -/// +/// /// ``` -/// +/// /// The iterator yields tuples of (field: RedisString, value: RedisString). -/// +/// /// ## Implementation notes -/// +/// /// The `RedisModule_ScanKey` function from the C API uses a callback to return the field and value strings. We /// distinguish two cases: -/// -/// 1. Either the callback is called once, -/// 2. or multiple times -/// +/// +/// 1. Either the callback is called once, +/// 2. or multiple times +/// /// and this depends if a rehash happens during the scan. pub struct ScanKeyCursor { - key: RedisKey, + key: RedisKey, inner_cursor: *mut raw::RedisModuleScanCursor, } @@ -94,14 +97,8 @@ enum IteratorState { HasBufferedItems, Done, } - -enum StackSlotState { - Empty, - Filled(ScanKeyIteratorItem), -} - struct StackSlot<'a> { - state: StackSlotState, + //state: StackSlotState, ctx: Context, buf: &'a mut Vec, } @@ -119,12 +116,10 @@ impl ScanKeyCursorIterator<'_> { fn next_scan_call(&mut self) -> Option { let ctx_ptr = self.cursor.key.ctx; - let ctx = Context::new(ctx_ptr); - + let mut stack_slot = StackSlot { - state: StackSlotState::Empty, ctx: Context::new(ctx_ptr), - buf: &mut self.buf + buf: &mut self.buf, }; let data_ptr = addr_of_mut!(stack_slot).cast::(); @@ -149,14 +144,12 @@ impl ScanKeyCursorIterator<'_> { // we may still have buffered items } - let StackSlotState::Filled(reval) = stack_slot.state else { - // should not happen - panic!("ScanKey callback did not fill the stack slot"); - }; - - ctx.log_notice(&format!("next Reval field: {}, value: {}", reval.0, reval.1)); - - Some(reval) + if stack_slot.buf.is_empty() { + // no items were returned, try again + None + } else { + self.next_buffered_item() + } } fn next_buffered_item(&mut self) -> Option { @@ -166,7 +159,7 @@ impl ScanKeyCursorIterator<'_> { } /// The callback that is called by `RedisModule_ScanKey` to return the field and value strings. -/// +/// /// The `data` pointer is a stack slot of type `RawData` that is used to pass the data back to the iterator. unsafe extern "C" fn foreach_callback( key: *mut raw::RedisModuleKey, @@ -182,7 +175,7 @@ unsafe extern "C" fn foreach_callback()) }; callback(&key, &field, &value); - + // we're not the owner of field and value strings field.take(); value.take(); @@ -191,7 +184,7 @@ unsafe extern "C" fn foreach_callback { - let out = format!("CB - Fill empty slot - Field: {}, Value: {}", field, value); - slot.ctx.log_notice(&out); - slot.state = StackSlotState::Filled((field, value)); - } - StackSlotState::Filled(_) => { - // This is the case where the callback is called multiple times. - // We need to buffer the data in the iterator state. - let out = format!("CB - Buffer for future use - Field: {}, Value: {}", field, value); - slot.ctx.log_notice(&out); - slot.buf.push((field, value)); - - } + if slot.buf.is_empty() { + let out = format!("CB - Value tu return - Field: {}, Value: {}", field, value); + slot.ctx.log_notice(&out); + } else { + // This is the case where the callback is called multiple times. + // We need to buffer the data in the iterator state. + let out = format!( + "CB - Buffer for future use - Field: {}, Value: {}", + field, value + ); + slot.ctx.log_notice(&out); } - + slot.buf.push((field, value)); } // Implements an iterator for `KeysCursor` that yields (RedisKey, *mut RedisModuleString, *mut RedisModuleString) in a Rust for loop. @@ -244,4 +234,4 @@ impl Iterator for ScanKeyCursorIterator<'_> { IteratorState::Done => None, } } -} \ No newline at end of file +} From 05fe1fa0c8027029cbd4ceded3c1c6bebe0013ef Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Thu, 28 Aug 2025 13:03:40 +0200 Subject: [PATCH 05/17] documentation --- src/context/key_scan_cursor.rs | 79 ++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 19 deletions(-) diff --git a/src/context/key_scan_cursor.rs b/src/context/key_scan_cursor.rs index 8ca45a5c..cd1229c1 100644 --- a/src/context/key_scan_cursor.rs +++ b/src/context/key_scan_cursor.rs @@ -5,34 +5,70 @@ use std::{ use crate::{key::RedisKey, raw, Context, RedisString}; -/// A cursor to scan fields and values in a hash key. +/// A cursor to scan field/value pairs of a (hash) key. /// -/// This is a wrapper around the RedisModule_ScanKey function from the C API. It provides access via [`ScanKeyCursor::foreach] and provides -/// a Rust iterator. +/// This is a wrapper around the [RedisModule_ScanKey](https://redis.io/docs/latest/develop/reference/modules/modules-api-ref/#redismodule_scankey) +/// function from the C API. It provides access via [`ScanKeyCursor::foreach`] and provides a Rust iterator via [`ScanKeyCursor::iter`]. +/// +/// Use the former if the operation can be performed in the callback, as it is more efficient. Use the latter if you need to collect the results and/or +/// want to have access to the Rust iterator API. /// -/// Example usage: +/// ## Example usage +/// +/// Here we show how to extract values to communicate them back to the Redis client. We assume that the following hash key is setup: +/// /// ```no_run -/// +/// HSET user:123 name Alice age 29 location Austin /// ``` +/// +/// For using the `foreach` method: +/// +/// ```no_run +/// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { +/// let key = ctx.open_key_with_flags("user:123", KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); +/// let cursor = ScanKeyCursor::new(key); +/// +/// let res = RefCell::new(Vec::new()); +/// cursor.foreach(|_key, field, value| { +/// let mut res = res.borrow_mut(); +/// res.push(RedisValue::BulkRedisString(field.clone())); +/// res.push(RedisValue::BulkRedisString(value.clone())); +/// }); /// -/// The iterator yields tuples of (field: RedisString, value: RedisString). -/// -/// ## Implementation notes -/// -/// The `RedisModule_ScanKey` function from the C API uses a callback to return the field and value strings. We -/// distinguish two cases: -/// -/// 1. Either the callback is called once, -/// 2. or multiple times -/// -/// and this depends if a rehash happens during the scan. +/// Ok(RedisValue::Array(res.take())) +/// } +/// ``` +/// +/// For using the `iter` method: +/// +/// ```no_run +/// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { +/// let mut res = Vec::new(); +/// let key = ctx.open_key_with_flags("user:123", KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); +/// let cursor = ScanKeyCursor::new(key); +/// for (field, value) in cursor.iter().enumerate() { +/// res.push(RedisValue::BulkRedisString(field)); +/// res.push(RedisValue::BulkRedisString(value)); +/// } +/// Ok(RedisValue::Array(res)) +/// } +/// ``` +/// +/// Both methods will produce the following output: +/// +/// ```text +/// 1) "name" +/// 2) "Alice" +/// 3) "age" +/// 4) "29" +/// 5) "location" +/// 6) "Austin" +/// ``` pub struct ScanKeyCursor { key: RedisKey, inner_cursor: *mut raw::RedisModuleScanCursor, } -//type ScanKeyCallback = F where F: FnMut(&RedisKey, &RedisString, &RedisString); - impl ScanKeyCursor { pub fn new(key: RedisKey) -> Self { let inner_cursor = unsafe { raw::RedisModule_ScanCursorCreate.unwrap()() }; @@ -85,6 +121,7 @@ pub struct ScanKeyCursorIterator<'a> { /// The cursor that is used for the iteration cursor: &'a ScanKeyCursor, + // todo: use a vector with stack allocation for better performance /// Buffer to hold the uninitialized data if the C callback is called multiple times. buf: Vec, @@ -92,13 +129,17 @@ pub struct ScanKeyCursorIterator<'a> { last_call: bool, } +/// The state machine for the iterator enum IteratorState { NeedToCallScanKey, HasBufferedItems, Done, } + +/// A stack slot that is used to pass data from the C callback to the iterator. +/// +/// It is mainly used to access the context and to store the buffered items. struct StackSlot<'a> { - //state: StackSlotState, ctx: Context, buf: &'a mut Vec, } From c9f20c2ff239f8df39425a57111eb63c5c2ba83e Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Thu, 28 Aug 2025 13:28:04 +0200 Subject: [PATCH 06/17] cleanup and integration testing --- Cargo.toml | 4 --- build.rs | 8 ----- examples/scan_keys.rs | 52 ++--------------------------- src/context/key_scan_cursor.rs | 60 ++++++++++++++++++---------------- src/key.rs | 8 ++--- src/raw.rs | 16 ++++----- src/redismodule.rs | 4 --- tests/integration.rs | 35 ++++++++++++++++++++ 8 files changed, 80 insertions(+), 107 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b1aba904..d02f5191 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -147,10 +147,6 @@ redis-module = { path = "./", default-features = false } bindgen = { version = "0.66", default-features = false, features = ["logging", "prettyplease"]} cc = "1" -#[build-dependencies.bindgen] -#version = "0.71.1" -#default-features = false - [features] default = ["min-redis-compatibility-version-6-0", "bindgen-runtime"] diff --git a/build.rs b/build.rs index 4b655094..8258fd85 100644 --- a/build.rs +++ b/build.rs @@ -14,14 +14,6 @@ impl ParseCallbacks for RedisModuleCallback { Some(IntKind::U64) } else if name.starts_with("REDISMODULE_REPLY") { Some(IntKind::I32) - } else if name == "REDISMODULE_LIST_HEAD" - || name == "REDISMODULE_LIST_TAIL" - { - // These values are used as `enum` discriminants, and thus must be `isize`. - Some(IntKind::Custom { - name: "isize", - is_signed: true, - }) } else if name.starts_with("REDISMODULE_NOTIFY_") { Some(IntKind::Int) } else { diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index c8e11521..d8329956 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -11,42 +11,6 @@ use std::cell::RefCell; // `scan_key_it` always copies the field and value strings, while `scan_key_fe` uses references to the field and value strings. In that example // both implementations need to clone the strings, because we want to return them as an array of RedisString. // -// ## Usage examples for scan_key: -// -// First we need to setup a hash key with some fields and values: -// -// ``` -// HSET user:123 name Alice age 29 location Austin -// ``` -// -// We need to start redis-server with the module loaded (example for macOS, adjust path as needed): -// -// ``` -// redis-server --loadmodule ./target/debug/examples/libscan_keys.dylib -// ``` -// -// Afterwards we can call the commands via redis-cli: -// -// ``` -// > SCAN_KEYS -// 1) "user:123" -// -// > SCAN_KEY_IT user:123 -// 1) "name" -// 2) "Alice" -// 3) "age" -// 4) "29" -// 5) "location" -// 6) "Austin" -// -// > SCAN_KEY_FE user:123 -// 1) "name" -// 2) "Alice" -// 3) "age" -// 4) "29" -// 5) "location" -// 6) "Austin" -// ``` use redis_module::{ key::{KeyFlags, RedisKey}, redis_module, Context, KeysCursor, RedisError, RedisResult, RedisString, RedisValue, ScanKeyCursor @@ -77,16 +41,10 @@ fn scan_key_foreach(ctx: &Context, args: Vec) -> RedisResult { let key_name = &args[1]; let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); - let ty = key.key_type(); let cursor = ScanKeyCursor::new(key); - let out = format!("Cursor created for Scanning key: {}, type={:?}", key_name, ty); - ctx.log_notice(&out); - + let res = RefCell::new(Vec::new()); cursor.foreach(|_key, field, value| { - let out = format!("Field: {}, Value: {}", field, value); - ctx.log_notice(&out); - let mut res = res.borrow_mut(); res.push(RedisValue::BulkRedisString(field.clone())); res.push(RedisValue::BulkRedisString(value.clone())); @@ -106,17 +64,13 @@ fn scan_key_iterator(ctx: &Context, args: Vec) -> RedisResult { let key_name = &args[1]; let mut res = Vec::new(); let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); - let ty = key.key_type(); let cursor = ScanKeyCursor::new(key); - let out = format!("Cursor created for Scanning key: {}, type={:?}", key_name, ty); - ctx.log_notice(&out); - + for (no, (field, value)) in cursor.iter().enumerate() { - let out = format!("It: {}, Field: {}, Value: {}", no, field, value); - ctx.log_notice(&out); res.push(RedisValue::BulkRedisString(field)); res.push(RedisValue::BulkRedisString(value)); } + Ok(RedisValue::Array(res)) } diff --git a/src/context/key_scan_cursor.rs b/src/context/key_scan_cursor.rs index cd1229c1..5b87e597 100644 --- a/src/context/key_scan_cursor.rs +++ b/src/context/key_scan_cursor.rs @@ -8,22 +8,23 @@ use crate::{key::RedisKey, raw, Context, RedisString}; /// A cursor to scan field/value pairs of a (hash) key. /// /// This is a wrapper around the [RedisModule_ScanKey](https://redis.io/docs/latest/develop/reference/modules/modules-api-ref/#redismodule_scankey) -/// function from the C API. It provides access via [`ScanKeyCursor::foreach`] and provides a Rust iterator via [`ScanKeyCursor::iter`]. +/// function from the C API. It provides access via a closure given to [`ScanKeyCursor::foreach`] and alternatively +/// provides a Rust iterator via [`ScanKeyCursor::iter`]. /// -/// Use the former if the operation can be performed in the callback, as it is more efficient. Use the latter if you need to collect the results and/or +/// Use `foreach` if the operation requires no copies and high performance. Use the iterator variant if you need to collect the results and/or /// want to have access to the Rust iterator API. /// /// ## Example usage /// /// Here we show how to extract values to communicate them back to the Redis client. We assume that the following hash key is setup: /// -/// ```no_run +/// ```text /// HSET user:123 name Alice age 29 location Austin /// ``` /// /// For using the `foreach` method: /// -/// ```no_run +/// ```ignore /// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { /// let key = ctx.open_key_with_flags("user:123", KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); /// let cursor = ScanKeyCursor::new(key); @@ -41,7 +42,7 @@ use crate::{key::RedisKey, raw, Context, RedisString}; /// /// For using the `iter` method: /// -/// ```no_run +/// ```ignore /// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { /// let mut res = Vec::new(); /// let key = ctx.open_key_with_flags("user:123", KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); @@ -70,16 +71,18 @@ pub struct ScanKeyCursor { } impl ScanKeyCursor { + /// Creates a new scan cursor for the given key. pub fn new(key: RedisKey) -> Self { let inner_cursor = unsafe { raw::RedisModule_ScanCursorCreate.unwrap()() }; Self { key, inner_cursor } } + /// Restarts the cursor from the beginning. pub fn restart(&self) { unsafe { raw::RedisModule_ScanCursorRestart.unwrap()(self.inner_cursor) }; } - /// Implements a callback based foreach loop over all fields and values in the hash key, use for optimal performance. + /// Implements a callback based foreach loop over all fields and values in the hash key, use that for optimal performance. pub fn foreach(&self, f: F) { // Safety: Assumption: c-side initialized the function ptr and it is is never changed, // i.e. after module initialization the function pointers stay valid till the end of the program. @@ -98,9 +101,12 @@ impl ScanKeyCursor { } } + /// Returns an iterator over all fields and values in the hash key. + /// + /// As the rust loop scope is detached from the C callback + /// we need to buffer the field/value pairs. That has performance implications. They are lower if the field/value pairs are + /// copied anyway, but even in that case not as fast as using the [`ScanKeyCursor::foreach`] method. pub fn iter(&self) -> ScanKeyCursorIterator<'_> { - let ctx = Context::new(self.key.ctx); - ctx.log_notice("Starting ScanKeyCursor iteration"); ScanKeyCursorIterator { cursor: self, buf: Vec::new(), @@ -117,6 +123,7 @@ impl Drop for ScanKeyCursor { pub type ScanKeyIteratorItem = (RedisString, RedisString); +/// An iterator (state) over the field/value pairs of a hash key. pub struct ScanKeyCursorIterator<'a> { /// The cursor that is used for the iteration cursor: &'a ScanKeyCursor, @@ -199,9 +206,10 @@ impl ScanKeyCursorIterator<'_> { } } -/// The callback that is called by `RedisModule_ScanKey` to return the field and value strings. +/// The callback that is used by [`ScanKeyCursor::foreach`] as argument to `RedisModule_ScanKey`. /// -/// The `data` pointer is a stack slot of type `RawData` that is used to pass the data back to the iterator. +/// The `data` pointer is the closure given to [`ScanKeyCursor::foreach`] and the callback forwards +/// references to the key, field and value to that closure. unsafe extern "C" fn foreach_callback( key: *mut raw::RedisModuleKey, field: *mut raw::RedisModuleString, @@ -224,43 +232,37 @@ unsafe extern "C" fn foreach_callback(); let slot = &mut (*slot); - // todo: use new-type with refcount handling for better performance + // todo: use new-type with refcount handling for better performance, otherwise we have to copy at this point + // we know that this callback will be called in a loop scope and that we + // need to store the RedisString longer than the ScanKey invocation but not much + // longer and in case of batched results we don't need to store everything in memory + // but only the last batch. let field = raw::RedisModule_CreateStringFromString.unwrap()(slot.ctx.get_raw(), field); let value = raw::RedisModule_CreateStringFromString.unwrap()(slot.ctx.get_raw(), value); let field = RedisString::from_redis_module_string(slot.ctx.get_raw(), field); let value = RedisString::from_redis_module_string(slot.ctx.get_raw(), value); - if slot.buf.is_empty() { - let out = format!("CB - Value tu return - Field: {}, Value: {}", field, value); - slot.ctx.log_notice(&out); - } else { - // This is the case where the callback is called multiple times. - // We need to buffer the data in the iterator state. - let out = format!( - "CB - Buffer for future use - Field: {}, Value: {}", - field, value - ); - slot.ctx.log_notice(&out); - } slot.buf.push((field, value)); } -// Implements an iterator for `KeysCursor` that yields (RedisKey, *mut RedisModuleString, *mut RedisModuleString) in a Rust for loop. +// Implements an iterator for `KeysCursor` that yields (RedisString, RedisString) in a Rust for loop. // This is a wrapper around the RedisModule_ScanKey function from the C API and uses a pattern to get the values from the callback that // is also used in stack unwinding scenarios. There is not common term for that but here we can think of it as a "stack slot" pattern. impl Iterator for ScanKeyCursorIterator<'_> { diff --git a/src/key.rs b/src/key.rs index 76b7b966..9e363a11 100644 --- a/src/key.rs +++ b/src/key.rs @@ -110,8 +110,7 @@ impl RedisKey { /// Will panic if `RedisModule_KeyType` is missing in redismodule.h #[must_use] pub fn key_type(&self) -> raw::KeyType { - let discriminant = unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }; - KeyType::from_repr(discriminant as u32).unwrap() + unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }.into() } /// Detects whether the key pointer given to us by Redis is null. @@ -380,8 +379,7 @@ impl RedisKeyWritable { /// Will panic if `RedisModule_KeyType` is missing in redismodule.h #[must_use] pub fn key_type(&self) -> raw::KeyType { - let discriminant = unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }; - KeyType::from_repr(discriminant as u32).unwrap() + unsafe { raw::RedisModule_KeyType.unwrap()(self.key_inner) }.into() } pub fn open_with_redis_string( @@ -672,7 +670,7 @@ fn to_raw_mode(mode: KeyMode) -> raw::KeyMode { /// Will panic if `RedisModule_KeyType` or `RedisModule_ModuleTypeGetType` are missing in redismodule.h #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn verify_type(key_inner: *mut raw::RedisModuleKey, redis_type: &RedisType) -> RedisResult { - let key_type: KeyType = KeyType::from_repr(unsafe { raw::RedisModule_KeyType.unwrap()(key_inner) as u32 }).unwrap(); + let key_type: KeyType = unsafe { raw::RedisModule_KeyType.unwrap()(key_inner) }.into(); if key_type != KeyType::Empty { // The key exists; check its type diff --git a/src/raw.rs b/src/raw.rs index 1e4b6cb5..f79ec786 100644 --- a/src/raw.rs +++ b/src/raw.rs @@ -29,13 +29,6 @@ bitflags! { pub struct KeyMode: c_int { const READ = REDISMODULE_READ as c_int; const WRITE = REDISMODULE_WRITE as c_int; - - const KEYOPEN_NOTOUCH = REDISMODULE_OPEN_KEY_NOTOUCH as c_int; - const KEYOPEN_NONOTIFY = REDISMODULE_OPEN_KEY_NONOTIFY as c_int; - const KEYOPEN_NOSTATS = REDISMODULE_OPEN_KEY_NOSTATS as c_int; - const KEYOPEN_NOEFFECTS = REDISMODULE_OPEN_KEY_NOEFFECTS as c_int; - const KEYOPEN_NOEXPIRE = REDISMODULE_OPEN_KEY_NOEXPIRE as c_int; - const KEYOPEN_ACCESSEXPIRED = REDISMODULE_OPEN_KEY_ACCESS_EXPIRED as c_int; } } @@ -60,7 +53,14 @@ pub enum KeyType { Stream = REDISMODULE_KEYTYPE_STREAM, } -#[derive(Primitive, Debug, PartialEq, Eq)] +impl From for KeyType { + fn from(v: c_int) -> Self { + Self::from_i32(v).unwrap() + } +} + +#[repr(u32)] +#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] pub enum Where { ListHead = REDISMODULE_LIST_HEAD, ListTail = REDISMODULE_LIST_TAIL, diff --git a/src/redismodule.rs b/src/redismodule.rs index 3ba12b22..adba5c0f 100644 --- a/src/redismodule.rs +++ b/src/redismodule.rs @@ -285,10 +285,6 @@ impl RedisString { impl Drop for RedisString { fn drop(&mut self) { if !self.inner.is_null() { - let ctx = Context::new(self.ctx); - let out = format!("Dropping RedisString '{}'", self.to_string_lossy()); - ctx.log_notice(&out); - unsafe { raw::RedisModule_FreeString.unwrap()(self.ctx, self.inner); } diff --git a/tests/integration.rs b/tests/integration.rs index f5a1795d..92a9fe01 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -200,6 +200,41 @@ fn test_scan() -> Result<()> { Ok(()) } +#[test] +fn test_scan_key_it() -> Result<()> { + let mut con = TestConnection::new("scan_keys"); + + redis::cmd("hset") + .arg(&[ + "user:123", "name", "Alice", "age", "29", "location", "Austin", + ]) + .query::<()>(&mut con) + .with_context(|| "failed to hset")?; + + let res: Vec = redis::cmd("scan_key_it") + .arg(&["user:123"]) + .query(&mut con)?; + assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); + Ok(()) +} + +#[test] +fn test_scan_key_foreach() -> Result<()> { + let mut con = TestConnection::new("scan_keys"); + redis::cmd("hset") + .arg(&[ + "user:123", "name", "Alice", "age", "29", "location", "Austin", + ]) + .query::<()>(&mut con) + .with_context(|| "failed to hset")?; + + let res: Vec = redis::cmd("scan_key_fe") + .arg(&["user:123"]) + .query(&mut con)?; + assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); + Ok(()) +} + #[test] fn test_stream_reader() -> Result<()> { let mut con = TestConnection::new("stream"); From 72e612ce0553245e8befe3a6f2dfaff2a472531e Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Thu, 28 Aug 2025 16:15:54 +0200 Subject: [PATCH 07/17] change encapsulation for usage in redisearch_rs --- examples/scan_keys.rs | 2 -- src/key.rs | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index d8329956..ca359bd3 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -1,5 +1,3 @@ -use std::cell::RefCell; - // This example shows the usage of the scan functionality of the Rust Redis Module API Wrapper. // // The example implements three commands: diff --git a/src/key.rs b/src/key.rs index 9e363a11..c16f3d0c 100644 --- a/src/key.rs +++ b/src/key.rs @@ -70,7 +70,7 @@ impl RedisKey { Self { ctx, key_inner } } - pub(crate) fn open_with_flags( + pub fn open_with_flags( ctx: *mut raw::RedisModuleCtx, key: &RedisString, flags: KeyFlags, @@ -80,7 +80,7 @@ impl RedisKey { Self { ctx, key_inner } } - pub(crate) const fn from_raw_parts( + pub const fn from_raw_parts( ctx: *mut raw::RedisModuleCtx, key_inner: *mut raw::RedisModuleKey, ) -> Self { @@ -206,7 +206,7 @@ impl RedisKeyWritable { Self { ctx, key_inner } } - pub(crate) fn open_with_flags( + pub fn open_with_flags( ctx: *mut raw::RedisModuleCtx, key: &RedisString, flags: KeyFlags, From 1a98bb50d1b0575d42ac95b4b75bfe2f79f5fc93 Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Thu, 28 Aug 2025 17:30:20 +0200 Subject: [PATCH 08/17] use FnMut use FnMut --- examples/scan_keys.rs | 5 ++--- src/context/key_scan_cursor.rs | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index ca359bd3..64e59cf5 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -41,14 +41,13 @@ fn scan_key_foreach(ctx: &Context, args: Vec) -> RedisResult { let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); let cursor = ScanKeyCursor::new(key); - let res = RefCell::new(Vec::new()); + let mut res = Vec::new(); cursor.foreach(|_key, field, value| { - let mut res = res.borrow_mut(); res.push(RedisValue::BulkRedisString(field.clone())); res.push(RedisValue::BulkRedisString(value.clone())); }); - Ok(RedisValue::Array(res.take())) + Ok(RedisValue::Array(res)) } /// Scans all fields and values in a hash key and returns them as an array of RedisString. diff --git a/src/context/key_scan_cursor.rs b/src/context/key_scan_cursor.rs index 5b87e597..c4cba8a7 100644 --- a/src/context/key_scan_cursor.rs +++ b/src/context/key_scan_cursor.rs @@ -83,7 +83,7 @@ impl ScanKeyCursor { } /// Implements a callback based foreach loop over all fields and values in the hash key, use that for optimal performance. - pub fn foreach(&self, f: F) { + pub fn foreach(&self, f: F) { // Safety: Assumption: c-side initialized the function ptr and it is is never changed, // i.e. after module initialization the function pointers stay valid till the end of the program. let scan_key = unsafe { raw::RedisModule_ScanKey.unwrap() }; @@ -210,7 +210,7 @@ impl ScanKeyCursorIterator<'_> { /// /// The `data` pointer is the closure given to [`ScanKeyCursor::foreach`] and the callback forwards /// references to the key, field and value to that closure. -unsafe extern "C" fn foreach_callback( +unsafe extern "C" fn foreach_callback( key: *mut raw::RedisModuleKey, field: *mut raw::RedisModuleString, value: *mut raw::RedisModuleString, From dd503b35b4ecf5f43b41f02fb4daf811cb88141b Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Thu, 28 Aug 2025 18:14:46 +0200 Subject: [PATCH 09/17] expose inner of CallReply --- src/context/call_reply.rs | 78 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/src/context/call_reply.rs b/src/context/call_reply.rs index 6c739b6f..a6631d21 100644 --- a/src/context/call_reply.rs +++ b/src/context/call_reply.rs @@ -31,6 +31,11 @@ impl<'root> StringCallReply<'root> { }; unsafe { slice::from_raw_parts(reply_string, len) } } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } impl<'root> Drop for StringCallReply<'root> { @@ -77,6 +82,11 @@ impl<'root> ErrorCallReply<'root> { }; unsafe { slice::from_raw_parts(reply_string, len) } } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } impl<'root> Drop for ErrorCallReply<'root> { @@ -150,6 +160,11 @@ impl<'root> I64CallReply<'root> { pub fn to_i64(&self) -> i64 { call_reply_integer(self.reply.as_ptr()) } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } impl<'root> Drop for I64CallReply<'root> { @@ -204,6 +219,11 @@ impl<'root> ArrayCallReply<'root> { pub fn len(&self) -> usize { call_reply_length(self.reply.as_ptr()) } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } pub struct ArrayCallReplyIterator<'root, 'curr> { @@ -254,6 +274,13 @@ pub struct NullCallReply<'root> { _dummy: PhantomData<&'root ()>, } +impl<'root> NullCallReply<'root> { + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } +} + impl<'root> Drop for NullCallReply<'root> { fn drop(&mut self) { free_call_reply(self.reply.as_ptr()); @@ -303,6 +330,11 @@ impl<'root> MapCallReply<'root> { pub fn len(&self) -> usize { call_reply_length(self.reply.as_ptr()) } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } pub struct MapCallReplyIterator<'root, 'curr> { @@ -384,6 +416,11 @@ impl<'root> SetCallReply<'root> { pub fn len(&self) -> usize { call_reply_length(self.reply.as_ptr()) } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } pub struct SetCallReplyIterator<'root, 'curr> { @@ -445,6 +482,11 @@ impl<'root> BoolCallReply<'root> { pub fn to_bool(&self) -> bool { call_reply_bool(self.reply.as_ptr()) } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } impl<'root> Drop for BoolCallReply<'root> { @@ -478,6 +520,11 @@ impl<'root> DoubleCallReply<'root> { pub fn to_double(&self) -> f64 { call_reply_double(self.reply.as_ptr()) } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } impl<'root> Drop for DoubleCallReply<'root> { @@ -512,6 +559,11 @@ impl<'root> BigNumberCallReply<'root> { pub fn to_string(&self) -> Option { call_reply_big_number(self.reply.as_ptr()) } + + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } } impl<'root> Drop for BigNumberCallReply<'root> { @@ -540,6 +592,13 @@ pub struct VerbatimStringCallReply<'root> { _dummy: PhantomData<&'root ()>, } +impl<'root> VerbatimStringCallReply<'root> { + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + self.reply.as_ptr() + } +} + /// RESP3 state that the verbatim string format must be of length 3. const VERBATIM_FORMAT_LENGTH: usize = 3; /// The string format of a verbatim string ([VerbatimStringCallReply]). @@ -639,6 +698,25 @@ pub enum CallReply<'root> { VerbatimString(VerbatimStringCallReply<'root>), } +impl<'root> CallReply<'root> { + /// Return the raw pointer to the underlying [RedisModuleCallReply]. + pub fn get_raw(&self) -> *mut RedisModuleCallReply { + match self { + CallReply::Unknown => std::ptr::null_mut(), + CallReply::I64(inner) => inner.get_raw(), + CallReply::String(inner) => inner.get_raw(), + CallReply::Array(inner) => inner.get_raw(), + CallReply::Null(inner) => inner.get_raw(), + CallReply::Map(inner) => inner.get_raw(), + CallReply::Set(inner) => inner.get_raw(), + CallReply::Bool(inner) => inner.get_raw(), + CallReply::Double(inner) => inner.get_raw(), + CallReply::BigNumber(inner) => inner.get_raw(), + CallReply::VerbatimString(inner) => inner.get_raw(), + } + } +} + /// Send implementation to [CallReply]. /// We need to implements this trait because [CallReply] hold /// raw pointers to C data which does not auto implement the [Send] trait. From 2ae50c84a167b0cf8f3e9b7816257f3b782fcf26 Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Mon, 1 Sep 2025 11:11:16 +0200 Subject: [PATCH 10/17] remove breaking changes --- Cargo.toml | 1 - build.rs | 15 +++++++++++++-- src/raw.rs | 15 +++++---------- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d02f5191..b084c3cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -133,7 +133,6 @@ serde = { version = "1", features = ["derive"] } nix = { version = "0.26", default-features = false } cfg-if = "1" redis-module-macros-internals = { path = "./redismodule-rs-macros-internals" } -strum = {version = "0.27.2",features = ["derive"]} log = "0.4" [dev-dependencies] diff --git a/build.rs b/build.rs index 8258fd85..707d99ad 100644 --- a/build.rs +++ b/build.rs @@ -12,8 +12,19 @@ impl ParseCallbacks for RedisModuleCallback { fn int_macro(&self, name: &str, _value: i64) -> Option { if name.starts_with("REDISMODULE_SUBEVENT_") || name.starts_with("REDISMODULE_EVENT_") { Some(IntKind::U64) - } else if name.starts_with("REDISMODULE_REPLY") { - Some(IntKind::I32) + } else if name.starts_with("REDISMODULE_REPLY_") + || name.starts_with("REDISMODULE_KEYTYPE_") + || name.starts_with("REDISMODULE_AUX_") + || name == "REDISMODULE_OK" + || name == "REDISMODULE_ERR" + || name == "REDISMODULE_LIST_HEAD" + || name == "REDISMODULE_LIST_TAIL" + { + // These values are used as `enum` discriminants, and thus must be `isize`. + Some(IntKind::Custom { + name: "isize", + is_signed: true, + }) } else if name.starts_with("REDISMODULE_NOTIFY_") { Some(IntKind::Int) } else { diff --git a/src/raw.rs b/src/raw.rs index f79ec786..71622f3f 100644 --- a/src/raw.rs +++ b/src/raw.rs @@ -40,8 +40,7 @@ bitflags! { } } -#[repr(u32)] -#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] +#[derive(Primitive, Debug, PartialEq, Eq)] pub enum KeyType { Empty = REDISMODULE_KEYTYPE_EMPTY, String = REDISMODULE_KEYTYPE_STRING, @@ -59,15 +58,13 @@ impl From for KeyType { } } -#[repr(u32)] -#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] +#[derive(Primitive, Debug, PartialEq, Eq)] pub enum Where { ListHead = REDISMODULE_LIST_HEAD, ListTail = REDISMODULE_LIST_TAIL, } -#[repr(i32)] -#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] +#[derive(Primitive, Debug, PartialEq, Eq)] pub enum ReplyType { Unknown = REDISMODULE_REPLY_UNKNOWN, String = REDISMODULE_REPLY_STRING, @@ -89,15 +86,13 @@ impl From for ReplyType { } } -#[repr(u32)] -#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] +#[derive(Primitive, Debug, PartialEq, Eq)] pub enum Aux { Before = REDISMODULE_AUX_BEFORE_RDB, After = REDISMODULE_AUX_AFTER_RDB, } -#[repr(u32)] -#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)] +#[derive(Primitive, Debug, PartialEq, Eq)] pub enum Status { Ok = REDISMODULE_OK, Err = REDISMODULE_ERR, From 03682b7a23c65f1e3fb9a0af7e64c55181526acb Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Mon, 1 Sep 2025 12:16:08 +0200 Subject: [PATCH 11/17] rename file --- src/context/{key_scan_cursor.rs => key_cursor.rs} | 0 src/context/mod.rs | 2 +- src/lib.rs | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/context/{key_scan_cursor.rs => key_cursor.rs} (100%) diff --git a/src/context/key_scan_cursor.rs b/src/context/key_cursor.rs similarity index 100% rename from src/context/key_scan_cursor.rs rename to src/context/key_cursor.rs diff --git a/src/context/mod.rs b/src/context/mod.rs index e2ebe629..c1818171 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -32,7 +32,7 @@ pub mod call_reply; pub mod commands; pub mod defrag; pub mod info; -pub mod key_scan_cursor; +pub mod key_cursor; pub mod keys_cursor; pub mod server_events; pub mod thread_safe; diff --git a/src/lib.rs b/src/lib.rs index 4b8f28fe..21e4c17c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,8 +31,8 @@ pub use crate::context::call_reply::FutureCallReply; pub use crate::context::call_reply::{CallReply, CallResult, ErrorReply, PromiseCallReply}; pub use crate::context::commands; pub use crate::context::defrag; +pub use crate::context::key_cursor::ScanKeyCursor; pub use crate::context::keys_cursor::KeysCursor; -pub use crate::context::key_scan_cursor::ScanKeyCursor; pub use crate::context::server_events; pub use crate::context::AclCategory; pub use crate::context::AclPermissions; From 10d1ced7b1b3b1fd2b6345db767caf851b5d0ebb Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Tue, 2 Sep 2025 09:34:13 +0200 Subject: [PATCH 12/17] remove iterator method --- examples/scan_keys.rs | 30 +------ src/context/key_cursor.rs | 176 ++------------------------------------ tests/integration.rs | 18 ---- 3 files changed, 8 insertions(+), 216 deletions(-) diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index 64e59cf5..97e9134d 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -3,12 +3,7 @@ // The example implements three commands: // // 1. `scan_keys` - scans all keys in the database and returns their names as an array of RedisString. -// 2. `scan_key_it ` - scans all fields and values in a hash key providing an iterator and return the field/value pairs as an array of RedisString. -// 3. `scan_key_fe ` - scans all fields and values in a hash key using a closure that stores tthe field/value pairs as an array of RedisString. -// -// `scan_key_it` always copies the field and value strings, while `scan_key_fe` uses references to the field and value strings. In that example -// both implementations need to clone the strings, because we want to return them as an array of RedisString. -// +// 2. `scan_key ` - scans all fields and values in a hash key using a closure that stores the field/value pairs as an array of RedisString. use redis_module::{ key::{KeyFlags, RedisKey}, redis_module, Context, KeysCursor, RedisError, RedisResult, RedisString, RedisValue, ScanKeyCursor @@ -50,26 +45,6 @@ fn scan_key_foreach(ctx: &Context, args: Vec) -> RedisResult { Ok(RedisValue::Array(res)) } -/// Scans all fields and values in a hash key and returns them as an array of RedisString. -/// The command takes one argument: the name of the hash key to scan. -fn scan_key_iterator(ctx: &Context, args: Vec) -> RedisResult { - // only argument is the key name - if args.len() != 2 { - return Err(RedisError::WrongArity); - } - - let key_name = &args[1]; - let mut res = Vec::new(); - let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); - let cursor = ScanKeyCursor::new(key); - - for (no, (field, value)) in cursor.iter().enumerate() { - res.push(RedisValue::BulkRedisString(field)); - res.push(RedisValue::BulkRedisString(value)); - } - - Ok(RedisValue::Array(res)) -} ////////////////////////////////////////////////////// @@ -80,7 +55,6 @@ redis_module! { data_types: [], commands: [ ["scan_keys", scan_keys, "readonly", 0, 0, 0, ""], - ["scan_key_it", scan_key_iterator, "readonly", 0, 0, 0, ""], - ["scan_key_fe", scan_key_foreach, "readonly", 0, 0, 0, ""], + ["scan_key", scan_key_foreach, "readonly", 0, 0, 0, ""], ], } diff --git a/src/context/key_cursor.rs b/src/context/key_cursor.rs index c4cba8a7..2d3225e5 100644 --- a/src/context/key_cursor.rs +++ b/src/context/key_cursor.rs @@ -1,28 +1,24 @@ use std::{ ffi::c_void, - ptr::{self, addr_of_mut}, + ptr::{self}, }; -use crate::{key::RedisKey, raw, Context, RedisString}; +use crate::{key::RedisKey, raw, RedisString}; /// A cursor to scan field/value pairs of a (hash) key. /// /// This is a wrapper around the [RedisModule_ScanKey](https://redis.io/docs/latest/develop/reference/modules/modules-api-ref/#redismodule_scankey) -/// function from the C API. It provides access via a closure given to [`ScanKeyCursor::foreach`] and alternatively -/// provides a Rust iterator via [`ScanKeyCursor::iter`]. +/// function from the C API. It provides access via a closure given to [`ScanKeyCursor::foreach`] /// -/// Use `foreach` if the operation requires no copies and high performance. Use the iterator variant if you need to collect the results and/or -/// want to have access to the Rust iterator API. -/// /// ## Example usage /// -/// Here we show how to extract values to communicate them back to the Redis client. We assume that the following hash key is setup: +/// Here we show how to extract values to communicate them back to the Redis client. We assume that the following hash key is setup before: /// /// ```text /// HSET user:123 name Alice age 29 location Austin /// ``` /// -/// For using the `foreach` method: +/// The following example command implementation scans all fields and values in the hash key and returns them as an array of RedisString. /// /// ```ignore /// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { @@ -40,22 +36,7 @@ use crate::{key::RedisKey, raw, Context, RedisString}; /// } /// ``` /// -/// For using the `iter` method: -/// -/// ```ignore -/// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { -/// let mut res = Vec::new(); -/// let key = ctx.open_key_with_flags("user:123", KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); -/// let cursor = ScanKeyCursor::new(key); -/// for (field, value) in cursor.iter().enumerate() { -/// res.push(RedisValue::BulkRedisString(field)); -/// res.push(RedisValue::BulkRedisString(value)); -/// } -/// Ok(RedisValue::Array(res)) -/// } -/// ``` -/// -/// Both methods will produce the following output: +/// The method will produce the following output: /// /// ```text /// 1) "name" @@ -100,19 +81,6 @@ impl ScanKeyCursor { } } } - - /// Returns an iterator over all fields and values in the hash key. - /// - /// As the rust loop scope is detached from the C callback - /// we need to buffer the field/value pairs. That has performance implications. They are lower if the field/value pairs are - /// copied anyway, but even in that case not as fast as using the [`ScanKeyCursor::foreach`] method. - pub fn iter(&self) -> ScanKeyCursorIterator<'_> { - ScanKeyCursorIterator { - cursor: self, - buf: Vec::new(), - last_call: false, - } - } } impl Drop for ScanKeyCursor { @@ -121,91 +89,6 @@ impl Drop for ScanKeyCursor { } } -pub type ScanKeyIteratorItem = (RedisString, RedisString); - -/// An iterator (state) over the field/value pairs of a hash key. -pub struct ScanKeyCursorIterator<'a> { - /// The cursor that is used for the iteration - cursor: &'a ScanKeyCursor, - - // todo: use a vector with stack allocation for better performance - /// Buffer to hold the uninitialized data if the C callback is called multiple times. - buf: Vec, - - /// Stores a flag that indicates if scan_key needs to be called again - last_call: bool, -} - -/// The state machine for the iterator -enum IteratorState { - NeedToCallScanKey, - HasBufferedItems, - Done, -} - -/// A stack slot that is used to pass data from the C callback to the iterator. -/// -/// It is mainly used to access the context and to store the buffered items. -struct StackSlot<'a> { - ctx: Context, - buf: &'a mut Vec, -} - -impl ScanKeyCursorIterator<'_> { - fn current_state(&self) -> IteratorState { - if !self.buf.is_empty() { - IteratorState::HasBufferedItems - } else if self.last_call { - IteratorState::Done - } else { - IteratorState::NeedToCallScanKey - } - } - - fn next_scan_call(&mut self) -> Option { - let ctx_ptr = self.cursor.key.ctx; - - let mut stack_slot = StackSlot { - ctx: Context::new(ctx_ptr), - buf: &mut self.buf, - }; - - let data_ptr = addr_of_mut!(stack_slot).cast::(); - - // Safety: Assumption: c-side initialized the function ptr and it is is never changed, - // i.e. after module initialization the function pointers stay valid till the end of the program. - let scan_key = unsafe { raw::RedisModule_ScanKey.unwrap() }; - - // Safety: All pointers we pass here are guaranteed to remain valid during the `scan_key` call. - let ret = unsafe { - scan_key( - self.cursor.key.key_inner, - self.cursor.inner_cursor, - Some(iterator_callback), - data_ptr, - ) - }; - - // Check if more calls are needed - if ret == 0 { - self.last_call = true; - // we may still have buffered items - } - - if stack_slot.buf.is_empty() { - // no items were returned, try again - None - } else { - self.next_buffered_item() - } - } - - fn next_buffered_item(&mut self) -> Option { - // todo: use different datatype / access pattern for performance - Some(self.buf.remove(0)) - } -} - /// The callback that is used by [`ScanKeyCursor::foreach`] as argument to `RedisModule_ScanKey`. /// /// The `data` pointer is the closure given to [`ScanKeyCursor::foreach`] and the callback forwards @@ -231,50 +114,3 @@ unsafe extern "C" fn foreach_callback(); - let slot = &mut (*slot); - - // todo: use new-type with refcount handling for better performance, otherwise we have to copy at this point - // we know that this callback will be called in a loop scope and that we - // need to store the RedisString longer than the ScanKey invocation but not much - // longer and in case of batched results we don't need to store everything in memory - // but only the last batch. - let field = raw::RedisModule_CreateStringFromString.unwrap()(slot.ctx.get_raw(), field); - let value = raw::RedisModule_CreateStringFromString.unwrap()(slot.ctx.get_raw(), value); - - let field = RedisString::from_redis_module_string(slot.ctx.get_raw(), field); - let value = RedisString::from_redis_module_string(slot.ctx.get_raw(), value); - - slot.buf.push((field, value)); -} - -// Implements an iterator for `KeysCursor` that yields (RedisString, RedisString) in a Rust for loop. -// This is a wrapper around the RedisModule_ScanKey function from the C API and uses a pattern to get the values from the callback that -// is also used in stack unwinding scenarios. There is not common term for that but here we can think of it as a "stack slot" pattern. -impl Iterator for ScanKeyCursorIterator<'_> { - type Item = ScanKeyIteratorItem; - - fn next(&mut self) -> Option { - let ctx = Context::new(self.cursor.key.ctx); - ctx.log_notice("ScanKeyCursorIterator next() called"); - match self.current_state() { - IteratorState::NeedToCallScanKey => self.next_scan_call(), - IteratorState::HasBufferedItems => self.next_buffered_item(), - IteratorState::Done => None, - } - } -} diff --git a/tests/integration.rs b/tests/integration.rs index 92a9fe01..cbe96317 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -200,24 +200,6 @@ fn test_scan() -> Result<()> { Ok(()) } -#[test] -fn test_scan_key_it() -> Result<()> { - let mut con = TestConnection::new("scan_keys"); - - redis::cmd("hset") - .arg(&[ - "user:123", "name", "Alice", "age", "29", "location", "Austin", - ]) - .query::<()>(&mut con) - .with_context(|| "failed to hset")?; - - let res: Vec = redis::cmd("scan_key_it") - .arg(&["user:123"]) - .query(&mut con)?; - assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); - Ok(()) -} - #[test] fn test_scan_key_foreach() -> Result<()> { let mut con = TestConnection::new("scan_keys"); From b0746d364c28830222c8dc806393998975ed08dd Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Tue, 2 Sep 2025 09:53:52 +0200 Subject: [PATCH 13/17] add flexible scan method usable with a while loop and encapsulate callback, handle reaming review comments --- examples/scan_keys.rs | 27 +++++++++- src/context/key_cursor.rs | 102 ++++++++++++++++++++++---------------- tests/integration.rs | 21 +++++++- 3 files changed, 103 insertions(+), 47 deletions(-) diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index 97e9134d..26aad532 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -3,7 +3,8 @@ // The example implements three commands: // // 1. `scan_keys` - scans all keys in the database and returns their names as an array of RedisString. -// 2. `scan_key ` - scans all fields and values in a hash key using a closure that stores the field/value pairs as an array of RedisString. +// 2. `scan_key ` - scans all fields by using a closure and a while loop, thus allowing an early stop. Don't use the early stop but collects all the field/value pairs as an array of RedisString. +// 3. `scan_key_foreach ` - scans all fields and values in a hash key using a closure that stores the field/value pairs as an array of RedisString. use redis_module::{ key::{KeyFlags, RedisKey}, redis_module, Context, KeysCursor, RedisError, RedisResult, RedisString, RedisValue, ScanKeyCursor @@ -24,6 +25,27 @@ fn scan_keys(ctx: &Context, _args: Vec) -> RedisResult { Ok(RedisValue::Array(res)) } +fn scan_key(ctx: &Context, args: Vec) -> RedisResult { + // only argument is the key name + if args.len() != 2 { + return Err(RedisError::WrongArity); + } + + let key_name = &args[1]; + let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); + let cursor = ScanKeyCursor::new(key); + + let mut res = Vec::new(); + while cursor.scan(|_key, field, value| { + res.push(RedisValue::BulkRedisString(field.clone())); + res.push(RedisValue::BulkRedisString(value.clone())); + }) { + // here we could do something between scans if needed, like an early stop + } + + Ok(RedisValue::Array(res)) +} + /// Scans all fields and values in a hash key and returns them as an array of RedisString. /// The command takes one argument: the name of the hash key to scan. fn scan_key_foreach(ctx: &Context, args: Vec) -> RedisResult { @@ -55,6 +77,7 @@ redis_module! { data_types: [], commands: [ ["scan_keys", scan_keys, "readonly", 0, 0, 0, ""], - ["scan_key", scan_key_foreach, "readonly", 0, 0, 0, ""], + ["scan_key", scan_key, "readonly", 0, 0, 0, ""], + ["scan_key_foreach", scan_key_foreach, "readonly", 0, 0, 0, ""], ], } diff --git a/src/context/key_cursor.rs b/src/context/key_cursor.rs index 2d3225e5..f47d5991 100644 --- a/src/context/key_cursor.rs +++ b/src/context/key_cursor.rs @@ -7,19 +7,19 @@ use crate::{key::RedisKey, raw, RedisString}; /// A cursor to scan field/value pairs of a (hash) key. /// -/// This is a wrapper around the [RedisModule_ScanKey](https://redis.io/docs/latest/develop/reference/modules/modules-api-ref/#redismodule_scankey) -/// function from the C API. It provides access via a closure given to [`ScanKeyCursor::foreach`] -/// +/// It provides access via a closure given to [`ScanKeyCursor::foreach`] or if you need more control, you can use [`ScanKeyCursor::scan`] +/// and implement your own loop, e.g. to allow an early stop. +/// /// ## Example usage -/// +/// /// Here we show how to extract values to communicate them back to the Redis client. We assume that the following hash key is setup before: -/// +/// /// ```text /// HSET user:123 name Alice age 29 location Austin /// ``` -/// +/// /// The following example command implementation scans all fields and values in the hash key and returns them as an array of RedisString. -/// +/// /// ```ignore /// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { /// let key = ctx.open_key_with_flags("user:123", KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); @@ -35,9 +35,9 @@ use crate::{key::RedisKey, raw, RedisString}; /// Ok(RedisValue::Array(res.take())) /// } /// ``` -/// +/// /// The method will produce the following output: -/// +/// /// ```text /// 1) "name" /// 2) "Alice" @@ -63,22 +63,31 @@ impl ScanKeyCursor { unsafe { raw::RedisModule_ScanCursorRestart.unwrap()(self.inner_cursor) }; } - /// Implements a callback based foreach loop over all fields and values in the hash key, use that for optimal performance. - pub fn foreach(&self, f: F) { - // Safety: Assumption: c-side initialized the function ptr and it is is never changed, + pub fn scan(&self, f: F) -> bool { + // the following is the callback definition + // foreach `ScanKey` call the callback may be called multiple times + use pimpl::scan_callback; + + // Safety: The c-side initialized the function ptr and it is is never changed, // i.e. after module initialization the function pointers stay valid till the end of the program. - let scan_key = unsafe { raw::RedisModule_ScanKey.unwrap() }; + let res = unsafe { + raw::RedisModule_ScanKey.unwrap()( + self.key.key_inner, + self.inner_cursor, + Some(scan_callback::), + &f as *const F as *mut c_void, + ) + }; + + res != 0 + } - let mut res = 1; - while res != 0 { - res = unsafe { - scan_key( - self.key.key_inner, - self.inner_cursor, - Some(foreach_callback::), - &f as *const F as *mut c_void, - ) - } + /// Implements a callback based foreach loop over all fields and values in the hash key, use that for optimal performance. + pub fn foreach(&self, mut f: F) { + // the following is the callback definition + // foreach `ScanKey` call the callback may be called multiple times + while self.scan(&mut f) { + // do nothing, the callback does the work } } } @@ -89,28 +98,35 @@ impl Drop for ScanKeyCursor { } } -/// The callback that is used by [`ScanKeyCursor::foreach`] as argument to `RedisModule_ScanKey`. -/// -/// The `data` pointer is the closure given to [`ScanKeyCursor::foreach`] and the callback forwards -/// references to the key, field and value to that closure. -unsafe extern "C" fn foreach_callback( - key: *mut raw::RedisModuleKey, - field: *mut raw::RedisModuleString, - value: *mut raw::RedisModuleString, - data: *mut c_void, -) { - let ctx = ptr::null_mut(); - let key = RedisKey::from_raw_parts(ctx, key); +// the module contains the private implementation details of the cursor. +mod pimpl { + use super::*; + + /// The callback that is used by [`ScanKeyCursor::scan`] and [`ScanKeyCursor::foreach`] as argument to `RedisModule_ScanKey`. + /// + /// The `data` pointer is the closure given to [`ScanKeyCursor::foreach`] and the callback forwards + /// references to the key, field and value to that closure. + pub(super) unsafe extern "C" fn scan_callback< + F: FnMut(&RedisKey, &RedisString, &RedisString), + >( + key: *mut raw::RedisModuleKey, + field: *mut raw::RedisModuleString, + value: *mut raw::RedisModuleString, + data: *mut c_void, + ) { + let ctx = ptr::null_mut(); + let key = RedisKey::from_raw_parts(ctx, key); - let field = RedisString::from_redis_module_string(ctx, field); - let value = RedisString::from_redis_module_string(ctx, value); + let field = RedisString::from_redis_module_string(ctx, field); + let value = RedisString::from_redis_module_string(ctx, value); - let callback = unsafe { &mut *(data.cast::()) }; - callback(&key, &field, &value); + let callback = unsafe { &mut *(data.cast::()) }; + callback(&key, &field, &value); - // we're not the owner of field and value strings - field.take(); - value.take(); + // we're not the owner of field and value strings + field.take(); + value.take(); - key.take(); // we're not the owner of the key either + key.take(); // we're not the owner of the key either + } } diff --git a/tests/integration.rs b/tests/integration.rs index cbe96317..dbaaffca 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -201,7 +201,7 @@ fn test_scan() -> Result<()> { } #[test] -fn test_scan_key_foreach() -> Result<()> { +fn test_scan_key() -> Result<()> { let mut con = TestConnection::new("scan_keys"); redis::cmd("hset") .arg(&[ @@ -210,7 +210,24 @@ fn test_scan_key_foreach() -> Result<()> { .query::<()>(&mut con) .with_context(|| "failed to hset")?; - let res: Vec = redis::cmd("scan_key_fe") + let res: Vec = redis::cmd("scan_key") + .arg(&["user:123"]) + .query(&mut con)?; + assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); + Ok(()) +} + +#[test] +fn test_scan_key_for_each() -> Result<()> { + let mut con = TestConnection::new("scan_keys"); + redis::cmd("hset") + .arg(&[ + "user:123", "name", "Alice", "age", "29", "location", "Austin", + ]) + .query::<()>(&mut con) + .with_context(|| "failed to hset")?; + + let res: Vec = redis::cmd("scan_key_foreach") .arg(&["user:123"]) .query(&mut con)?; assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); From d23e1ac299ba5bb40f8da4b8df3f5b64d41e4ede Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Tue, 2 Sep 2025 11:34:17 +0200 Subject: [PATCH 14/17] `for_each` instead of `foreach` --- examples/scan_keys.rs | 8 ++++---- src/context/key_cursor.rs | 19 ++++++++----------- tests/integration.rs | 2 +- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index 26aad532..5149ac7d 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -4,7 +4,7 @@ // // 1. `scan_keys` - scans all keys in the database and returns their names as an array of RedisString. // 2. `scan_key ` - scans all fields by using a closure and a while loop, thus allowing an early stop. Don't use the early stop but collects all the field/value pairs as an array of RedisString. -// 3. `scan_key_foreach ` - scans all fields and values in a hash key using a closure that stores the field/value pairs as an array of RedisString. +// 3. `scan_key_for_each ` - scans all fields and values in a hash key using a closure that stores the field/value pairs as an array of RedisString. use redis_module::{ key::{KeyFlags, RedisKey}, redis_module, Context, KeysCursor, RedisError, RedisResult, RedisString, RedisValue, ScanKeyCursor @@ -48,7 +48,7 @@ fn scan_key(ctx: &Context, args: Vec) -> RedisResult { /// Scans all fields and values in a hash key and returns them as an array of RedisString. /// The command takes one argument: the name of the hash key to scan. -fn scan_key_foreach(ctx: &Context, args: Vec) -> RedisResult { +fn scan_key_for_each(ctx: &Context, args: Vec) -> RedisResult { // only argument is the key name if args.len() != 2 { return Err(RedisError::WrongArity); @@ -59,7 +59,7 @@ fn scan_key_foreach(ctx: &Context, args: Vec) -> RedisResult { let cursor = ScanKeyCursor::new(key); let mut res = Vec::new(); - cursor.foreach(|_key, field, value| { + cursor.for_each(|_key, field, value| { res.push(RedisValue::BulkRedisString(field.clone())); res.push(RedisValue::BulkRedisString(value.clone())); }); @@ -78,6 +78,6 @@ redis_module! { commands: [ ["scan_keys", scan_keys, "readonly", 0, 0, 0, ""], ["scan_key", scan_key, "readonly", 0, 0, 0, ""], - ["scan_key_foreach", scan_key_foreach, "readonly", 0, 0, 0, ""], + ["scan_key_for_each", scan_key_for_each, "readonly", 0, 0, 0, ""], ], } diff --git a/src/context/key_cursor.rs b/src/context/key_cursor.rs index f47d5991..b36857a5 100644 --- a/src/context/key_cursor.rs +++ b/src/context/key_cursor.rs @@ -7,7 +7,7 @@ use crate::{key::RedisKey, raw, RedisString}; /// A cursor to scan field/value pairs of a (hash) key. /// -/// It provides access via a closure given to [`ScanKeyCursor::foreach`] or if you need more control, you can use [`ScanKeyCursor::scan`] +/// It provides access via a closure given to [`ScanKeyCursor::for_each`] or if you need more control, you can use [`ScanKeyCursor::scan`] /// and implement your own loop, e.g. to allow an early stop. /// /// ## Example usage @@ -21,12 +21,12 @@ use crate::{key::RedisKey, raw, RedisString}; /// The following example command implementation scans all fields and values in the hash key and returns them as an array of RedisString. /// /// ```ignore -/// fn example_scan_key_foreach(ctx: &Context) -> RedisResult { +/// fn example_scan_key_for_each(ctx: &Context) -> RedisResult { /// let key = ctx.open_key_with_flags("user:123", KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); /// let cursor = ScanKeyCursor::new(key); /// /// let res = RefCell::new(Vec::new()); -/// cursor.foreach(|_key, field, value| { +/// cursor.for_each(|_key, field, value| { /// let mut res = res.borrow_mut(); /// res.push(RedisValue::BulkRedisString(field.clone())); /// res.push(RedisValue::BulkRedisString(value.clone())); @@ -64,8 +64,7 @@ impl ScanKeyCursor { } pub fn scan(&self, f: F) -> bool { - // the following is the callback definition - // foreach `ScanKey` call the callback may be called multiple times + // the following is the callback definition. The callback may be called multiple times per `RedisModule_ScanKey` invocation. use pimpl::scan_callback; // Safety: The c-side initialized the function ptr and it is is never changed, @@ -82,10 +81,8 @@ impl ScanKeyCursor { res != 0 } - /// Implements a callback based foreach loop over all fields and values in the hash key, use that for optimal performance. - pub fn foreach(&self, mut f: F) { - // the following is the callback definition - // foreach `ScanKey` call the callback may be called multiple times + /// Implements a callback based for_each loop over all fields and values in the hash key, use that for optimal performance. + pub fn for_each(&self, mut f: F) { while self.scan(&mut f) { // do nothing, the callback does the work } @@ -102,9 +99,9 @@ impl Drop for ScanKeyCursor { mod pimpl { use super::*; - /// The callback that is used by [`ScanKeyCursor::scan`] and [`ScanKeyCursor::foreach`] as argument to `RedisModule_ScanKey`. + /// The callback that is used by [`ScanKeyCursor::scan`] and [`ScanKeyCursor::for_each`] as argument to `RedisModule_ScanKey`. /// - /// The `data` pointer is the closure given to [`ScanKeyCursor::foreach`] and the callback forwards + /// The `data` pointer is the closure given to [`ScanKeyCursor::for_each`] and the callback forwards /// references to the key, field and value to that closure. pub(super) unsafe extern "C" fn scan_callback< F: FnMut(&RedisKey, &RedisString, &RedisString), diff --git a/tests/integration.rs b/tests/integration.rs index dbaaffca..614d6675 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -227,7 +227,7 @@ fn test_scan_key_for_each() -> Result<()> { .query::<()>(&mut con) .with_context(|| "failed to hset")?; - let res: Vec = redis::cmd("scan_key_foreach") + let res: Vec = redis::cmd("scan_key_for_each") .arg(&["user:123"]) .query(&mut con)?; assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); From ac223086060670733d190ea707a0cb8a1d4c762c Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Tue, 2 Sep 2025 11:58:50 +0200 Subject: [PATCH 15/17] move callback into function body --- src/context/key_cursor.rs | 65 +++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/src/context/key_cursor.rs b/src/context/key_cursor.rs index b36857a5..6f974753 100644 --- a/src/context/key_cursor.rs +++ b/src/context/key_cursor.rs @@ -7,7 +7,7 @@ use crate::{key::RedisKey, raw, RedisString}; /// A cursor to scan field/value pairs of a (hash) key. /// -/// It provides access via a closure given to [`ScanKeyCursor::for_each`] or if you need more control, you can use [`ScanKeyCursor::scan`] +/// It provides access via a closure given to [`ScanKeyCursor::for_each`] or if you need more control, you can use [`ScanKeyCursor::scan`] /// and implement your own loop, e.g. to allow an early stop. /// /// ## Example usage @@ -64,8 +64,34 @@ impl ScanKeyCursor { } pub fn scan(&self, f: F) -> bool { - // the following is the callback definition. The callback may be called multiple times per `RedisModule_ScanKey` invocation. - use pimpl::scan_callback; + // The following is the callback definition. The callback may be called multiple times per `RedisModule_ScanKey` invocation. + // The callback is used by [`ScanKeyCursor::scan`] and [`ScanKeyCursor::for_each`] as argument to `RedisModule_ScanKey`. + // + // The `data` pointer is the closure given to [`ScanKeyCursor::scan`] or [`ScanKeyCursor::for_each`]. + // The callback forwards references to the key, field and value to that closure. + unsafe extern "C" fn scan_callback< + F: FnMut(&RedisKey, &RedisString, &RedisString), + >( + key: *mut raw::RedisModuleKey, + field: *mut raw::RedisModuleString, + value: *mut raw::RedisModuleString, + data: *mut c_void, + ) { + let ctx = ptr::null_mut(); + let key = RedisKey::from_raw_parts(ctx, key); + + let field = RedisString::from_redis_module_string(ctx, field); + let value = RedisString::from_redis_module_string(ctx, value); + + let callback = unsafe { &mut *(data.cast::()) }; + callback(&key, &field, &value); + + // we're not the owner of field and value strings + field.take(); + value.take(); + + key.take(); // we're not the owner of the key either + } // Safety: The c-side initialized the function ptr and it is is never changed, // i.e. after module initialization the function pointers stay valid till the end of the program. @@ -94,36 +120,3 @@ impl Drop for ScanKeyCursor { unsafe { raw::RedisModule_ScanCursorDestroy.unwrap()(self.inner_cursor) }; } } - -// the module contains the private implementation details of the cursor. -mod pimpl { - use super::*; - - /// The callback that is used by [`ScanKeyCursor::scan`] and [`ScanKeyCursor::for_each`] as argument to `RedisModule_ScanKey`. - /// - /// The `data` pointer is the closure given to [`ScanKeyCursor::for_each`] and the callback forwards - /// references to the key, field and value to that closure. - pub(super) unsafe extern "C" fn scan_callback< - F: FnMut(&RedisKey, &RedisString, &RedisString), - >( - key: *mut raw::RedisModuleKey, - field: *mut raw::RedisModuleString, - value: *mut raw::RedisModuleString, - data: *mut c_void, - ) { - let ctx = ptr::null_mut(); - let key = RedisKey::from_raw_parts(ctx, key); - - let field = RedisString::from_redis_module_string(ctx, field); - let value = RedisString::from_redis_module_string(ctx, value); - - let callback = unsafe { &mut *(data.cast::()) }; - callback(&key, &field, &value); - - // we're not the owner of field and value strings - field.take(); - value.take(); - - key.take(); // we're not the owner of the key either - } -} From aff0e3b53351fda69135c0831772e4a5808831a3 Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Tue, 2 Sep 2025 16:35:27 +0200 Subject: [PATCH 16/17] review comments --- examples/scan_keys.rs | 25 ++++++++++++++++--------- src/context/key_cursor.rs | 29 +++++++++++++++++++---------- src/redismodule.rs | 14 ++++++++------ tests/integration.rs | 4 +--- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/examples/scan_keys.rs b/examples/scan_keys.rs index 5149ac7d..92e28369 100644 --- a/examples/scan_keys.rs +++ b/examples/scan_keys.rs @@ -1,13 +1,15 @@ // This example shows the usage of the scan functionality of the Rust Redis Module API Wrapper. -// +// // The example implements three commands: -// +// // 1. `scan_keys` - scans all keys in the database and returns their names as an array of RedisString. // 2. `scan_key ` - scans all fields by using a closure and a while loop, thus allowing an early stop. Don't use the early stop but collects all the field/value pairs as an array of RedisString. // 3. `scan_key_for_each ` - scans all fields and values in a hash key using a closure that stores the field/value pairs as an array of RedisString. use redis_module::{ - key::{KeyFlags, RedisKey}, redis_module, Context, KeysCursor, RedisError, RedisResult, RedisString, RedisValue, ScanKeyCursor + key::{KeyFlags, RedisKey}, + redis_module, Context, KeysCursor, RedisError, RedisResult, RedisString, RedisValue, + ScanKeyCursor, }; /// Scans all keys in the database and returns their names as an array of RedisString. @@ -32,8 +34,11 @@ fn scan_key(ctx: &Context, args: Vec) -> RedisResult { } let key_name = &args[1]; - let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); - let cursor = ScanKeyCursor::new(key); + let key = ctx.open_key_with_flags( + key_name, + KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED, + ); + let cursor = ScanKeyCursor::new(key); let mut res = Vec::new(); while cursor.scan(|_key, field, value| { @@ -55,9 +60,12 @@ fn scan_key_for_each(ctx: &Context, args: Vec) -> RedisResult { } let key_name = &args[1]; - let key = ctx.open_key_with_flags(key_name, KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED ); - let cursor = ScanKeyCursor::new(key); - + let key = ctx.open_key_with_flags( + key_name, + KeyFlags::NOEFFECTS | KeyFlags::NOEXPIRE | KeyFlags::ACCESS_EXPIRED, + ); + let cursor = ScanKeyCursor::new(key); + let mut res = Vec::new(); cursor.for_each(|_key, field, value| { res.push(RedisValue::BulkRedisString(field.clone())); @@ -67,7 +75,6 @@ fn scan_key_for_each(ctx: &Context, args: Vec) -> RedisResult { Ok(RedisValue::Array(res)) } - ////////////////////////////////////////////////////// redis_module! { diff --git a/src/context/key_cursor.rs b/src/context/key_cursor.rs index 6f974753..46f279ce 100644 --- a/src/context/key_cursor.rs +++ b/src/context/key_cursor.rs @@ -63,15 +63,21 @@ impl ScanKeyCursor { unsafe { raw::RedisModule_ScanCursorRestart.unwrap()(self.inner_cursor) }; } + /// Implements a call to `RedisModule_ScanKey` and calls the given closure for each callback invocation by ScanKey. + /// Returns `true` if there are more fields to scan, `false` otherwise. + /// + /// The callback may be called multiple times per `RedisModule_ScanKey` invocation. + /// + /// ## Example + /// + /// ``` + /// while cursor.scan(|_key, field, value| { + /// // do something with field and value + /// }) { + /// // do something between scans if needed, like an early stop + /// } pub fn scan(&self, f: F) -> bool { - // The following is the callback definition. The callback may be called multiple times per `RedisModule_ScanKey` invocation. - // The callback is used by [`ScanKeyCursor::scan`] and [`ScanKeyCursor::for_each`] as argument to `RedisModule_ScanKey`. - // - // The `data` pointer is the closure given to [`ScanKeyCursor::scan`] or [`ScanKeyCursor::for_each`]. - // The callback forwards references to the key, field and value to that closure. - unsafe extern "C" fn scan_callback< - F: FnMut(&RedisKey, &RedisString, &RedisString), - >( + unsafe extern "C" fn scan_callback( key: *mut raw::RedisModuleKey, field: *mut raw::RedisModuleString, value: *mut raw::RedisModuleString, @@ -95,8 +101,10 @@ impl ScanKeyCursor { // Safety: The c-side initialized the function ptr and it is is never changed, // i.e. after module initialization the function pointers stay valid till the end of the program. + let scan_key = unsafe { raw::RedisModule_ScanKey.unwrap() }; + let res = unsafe { - raw::RedisModule_ScanKey.unwrap()( + scan_key( self.key.key_inner, self.inner_cursor, Some(scan_callback::), @@ -107,7 +115,8 @@ impl ScanKeyCursor { res != 0 } - /// Implements a callback based for_each loop over all fields and values in the hash key, use that for optimal performance. + /// Implements a callback based for_each loop over all fields and values in the hash key. + /// If you need more control, e.g. stopping after a scan invocation, then use [`ScanKeyCursor::scan`] directly. pub fn for_each(&self, mut f: F) { while self.scan(&mut f) { // do nothing, the callback does the work diff --git a/src/redismodule.rs b/src/redismodule.rs index adba5c0f..7054535d 100644 --- a/src/redismodule.rs +++ b/src/redismodule.rs @@ -162,17 +162,19 @@ impl RedisString { } /// Create a RedisString from a raw C string and length. The String will be copied. - /// + /// /// # Safety - /// The caller must ensure that the provided pointer is valid and points to a memory region + /// The caller must ensure that the provided pointer is valid and points to a memory region /// that is at least `len` bytes long. #[allow(clippy::not_unsafe_ptr_arg_deref)] - pub unsafe fn from_raw_parts(ctx: Option>, s: *const c_char, len: libc::size_t) -> Self { + pub unsafe fn from_raw_parts( + ctx: Option>, + s: *const c_char, + len: libc::size_t, + ) -> Self { let ctx = ctx.map_or(std::ptr::null_mut(), |v| v.as_ptr()); - let inner = unsafe { - raw::RedisModule_CreateString.unwrap()(ctx, s, len) - }; + let inner = unsafe { raw::RedisModule_CreateString.unwrap()(ctx, s, len) }; Self { ctx, inner } } diff --git a/tests/integration.rs b/tests/integration.rs index 614d6675..9196b008 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -210,9 +210,7 @@ fn test_scan_key() -> Result<()> { .query::<()>(&mut con) .with_context(|| "failed to hset")?; - let res: Vec = redis::cmd("scan_key") - .arg(&["user:123"]) - .query(&mut con)?; + let res: Vec = redis::cmd("scan_key").arg(&["user:123"]).query(&mut con)?; assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); Ok(()) } From 3fc969bedb582e40af10d7376377c49bb87d2894 Mon Sep 17 00:00:00 2001 From: Tim Janus Date: Mon, 8 Sep 2025 10:12:45 +0200 Subject: [PATCH 17/17] small enhancements --- src/context/call_reply.rs | 26 +++++++++++++------------- src/context/key_cursor.rs | 2 +- src/redismodule.rs | 2 +- tests/integration.rs | 8 ++++++-- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/context/call_reply.rs b/src/context/call_reply.rs index a6631d21..8dc46044 100644 --- a/src/context/call_reply.rs +++ b/src/context/call_reply.rs @@ -699,20 +699,20 @@ pub enum CallReply<'root> { } impl<'root> CallReply<'root> { - /// Return the raw pointer to the underlying [RedisModuleCallReply]. - pub fn get_raw(&self) -> *mut RedisModuleCallReply { + /// Return the raw pointer to the underlying [RedisModuleCallReply], or `None` if this is the `Unknown` variant. + pub fn get_raw(&self) -> Option<*mut RedisModuleCallReply> { match self { - CallReply::Unknown => std::ptr::null_mut(), - CallReply::I64(inner) => inner.get_raw(), - CallReply::String(inner) => inner.get_raw(), - CallReply::Array(inner) => inner.get_raw(), - CallReply::Null(inner) => inner.get_raw(), - CallReply::Map(inner) => inner.get_raw(), - CallReply::Set(inner) => inner.get_raw(), - CallReply::Bool(inner) => inner.get_raw(), - CallReply::Double(inner) => inner.get_raw(), - CallReply::BigNumber(inner) => inner.get_raw(), - CallReply::VerbatimString(inner) => inner.get_raw(), + CallReply::Unknown => None, + CallReply::I64(inner) => Some(inner.get_raw()), + CallReply::String(inner) => Some(inner.get_raw()), + CallReply::Array(inner) => Some(inner.get_raw()), + CallReply::Null(inner) => Some(inner.get_raw()), + CallReply::Map(inner) => Some(inner.get_raw()), + CallReply::Set(inner) => Some(inner.get_raw()), + CallReply::Bool(inner) => Some(inner.get_raw()), + CallReply::Double(inner) => Some(inner.get_raw()), + CallReply::BigNumber(inner) => Some(inner.get_raw()), + CallReply::VerbatimString(inner) => Some(inner.get_raw()), } } } diff --git a/src/context/key_cursor.rs b/src/context/key_cursor.rs index 46f279ce..3e0684aa 100644 --- a/src/context/key_cursor.rs +++ b/src/context/key_cursor.rs @@ -70,7 +70,7 @@ impl ScanKeyCursor { /// /// ## Example /// - /// ``` + /// ```ignore /// while cursor.scan(|_key, field, value| { /// // do something with field and value /// }) { diff --git a/src/redismodule.rs b/src/redismodule.rs index 7054535d..b23cb1b7 100644 --- a/src/redismodule.rs +++ b/src/redismodule.rs @@ -161,7 +161,7 @@ impl RedisString { Self { ctx, inner } } - /// Create a RedisString from a raw C string and length. The String will be copied. + /// Create a RedisString from a raw C string and length. The provided C String will be copied. /// /// # Safety /// The caller must ensure that the provided pointer is valid and points to a memory region diff --git a/tests/integration.rs b/tests/integration.rs index 9196b008..793e2405 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -210,7 +210,10 @@ fn test_scan_key() -> Result<()> { .query::<()>(&mut con) .with_context(|| "failed to hset")?; - let res: Vec = redis::cmd("scan_key").arg(&["user:123"]).query(&mut con)?; + let res: Vec = redis::cmd("scan_key") + .arg(&["user:123"]) + .query(&mut con) + .with_context(|| "failed scan_key")?; assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); Ok(()) } @@ -227,7 +230,8 @@ fn test_scan_key_for_each() -> Result<()> { let res: Vec = redis::cmd("scan_key_for_each") .arg(&["user:123"]) - .query(&mut con)?; + .query(&mut con) + .with_context(|| "failed scan_key_for_each")?; assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]); Ok(()) }