cleanup and integration testing

Author: DarthB

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 {

src/context/key_scan_cursor.rs

@@ -99,8 +99,6 @@ impl ScanKeyCursor {
     }
 
     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 +115,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,
@@ -233,34 +232,25 @@ unsafe extern "C" fn iterator_callback(
     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
+    // `data` is a stack slot
     let slot = data.cast::<StackSlot>();
     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<'_> {

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,8 @@ pub enum KeyType {
     Stream = REDISMODULE_KEYTYPE_STREAM,
 }
 
-#[derive(Primitive, Debug, PartialEq, Eq)]
+#[repr(u32)]
+#[derive(Primitive, Debug, PartialEq, Eq, strum::FromRepr)]
 pub enum Where {
     ListHead = REDISMODULE_LIST_HEAD,
     ListTail = REDISMODULE_LIST_TAIL,

src/redismodule.rs

@@ -273,10 +273,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);
             }

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<String> = 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<String> = 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");