review comments

Author: DarthB

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 <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 <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
+    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<RedisString>) -> 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<RedisString>) -> 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<RedisString>) -> RedisResult {
     Ok(RedisValue::Array(res))
 }
 
-
 //////////////////////////////////////////////////////
 
 redis_module! {

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<F: FnMut(&RedisKey, &RedisString, &RedisString)>(&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<F: FnMut(&RedisKey, &RedisString, &RedisString)>(
             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::<F>),
@@ -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<F: FnMut(&RedisKey, &RedisString, &RedisString)>(&self, mut f: F) {
         while self.scan(&mut f) {
             // do nothing, the callback does the work

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<NonNull<raw::RedisModuleCtx>>, s: *const c_char, len: libc::size_t) -> Self {
+    pub unsafe fn from_raw_parts(
+        ctx: Option<NonNull<raw::RedisModuleCtx>>,
+        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 }
     }

tests/integration.rs

@@ -210,9 +210,7 @@ fn test_scan_key() -> Result<()> {
         .query::<()>(&mut con)
         .with_context(|| "failed to hset")?;
 
-    let res: Vec<String> = redis::cmd("scan_key")
-        .arg(&["user:123"])
-        .query(&mut con)?;
+    let res: Vec<String> = redis::cmd("scan_key").arg(&["user:123"]).query(&mut con)?;
     assert_eq!(&res, &["name", "Alice", "age", "29", "location", "Austin"]);
     Ok(())
 }