Use derive(Primite) for Redis module enums

Author: gavrie

Cargo.toml

@@ -20,6 +20,8 @@ crate-type = ["cdylib"]
 bitflags = "1.0"
 libc = "0.2"
 time = "0.1"
+enum-primitive-derive = "^0.1"
+num-traits = "^0.1"
 
 [build-dependencies]
 bindgen = "0.47"

examples/data_type.rs

@@ -14,8 +14,7 @@ use redismodule::types::RedisModuleType;
 const MODULE_NAME: &str = "alloc";
 const MODULE_VERSION: c_int = 1;
 
-// TODO: Can we use a safe smart pointer instead of an unsafe mutable static variable?
-static mut MY_TYPE: RedisModuleType = RedisModuleType::new();
+static MY_TYPE: RedisModuleType = RedisModuleType::new();
 
 struct AllocSetCommand;
 
@@ -29,6 +28,8 @@ impl Command for AllocSetCommand {
     // Run the command.
     fn run(r: Redis, args: &[&str]) -> Result<(), Error> {
         if args.len() != 3 {
+            // FIXME: Use RedisModule_WrongArity instead. Return an ArityError here and
+            // in the low-level implementation call RM_WrongArity.
             return Err(Error::generic(format!(
                 "Usage: {} <key> <size>", Self::name()
             ).as_str()));
@@ -43,8 +44,13 @@ impl Command for AllocSetCommand {
         // 2. Allocate data
         // 3. Set the key to the data
         // 4. Activate custom allocator and compare Redis memory usage
-        let data: Vec<u8> = Vec::with_capacity(size as usize);
-        let k = r.open_key_writable(key);
+
+        //let data: Vec<u8> = Vec::with_capacity(size as usize);
+
+        let key = r.open_key_writable(key);
+
+        //key.check_type(MY_TYPE)?;
+        key.write(size.to_string().as_str())?;
 
         /*
         raw::RedisModule_ModuleTypeSetValue.unwrap()(
@@ -113,8 +119,7 @@ pub extern "C" fn AllocDelCommand_Redis(
 fn module_on_load(ctx: *mut raw::RedisModuleCtx) -> Result<(), &'static str> {
     module_init(ctx, MODULE_NAME, MODULE_VERSION)?;
 
-    // FIXME: Make this safe (use a smart pointer?)
-    unsafe { MY_TYPE.create_data_type(ctx, "mytype123") }?;
+    MY_TYPE.create_data_type(ctx, "mytype123")?;
 
     AllocSetCommand::create(ctx)?;
     AllocDelCommand::create(ctx)?;

src/lib.rs

@@ -27,18 +27,22 @@ extern crate time;
 
 use libc::size_t;
 
+#[macro_use]
+extern crate enum_primitive_derive;
+extern crate num_traits;
+
 #[macro_use]
 mod macros;
 
 pub mod error;
 
-use crate::error::Error;
-
-////////////////////////////////////////////////////////////
 use std::alloc::{System, GlobalAlloc, Layout};
 use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering::SeqCst};
 use std::ffi::c_void;
 
+use crate::error::Error;
+use crate::types::RedisModuleType;
+
 struct RedisAlloc;
 
 static USE_REDIS_ALLOC: AtomicBool = ATOMIC_BOOL_INIT;
@@ -357,6 +361,20 @@ impl RedisKeyWritable {
             raw::Status::Err => Err(error!("Error while setting key")),
         }
     }
+
+    pub fn check_type(&self, redis_type: RedisModuleType) -> Result<(), Error> {
+        /*
+        int type = RedisModule_KeyType(key);
+        if (type != REDISMODULE_KEYTYPE_EMPTY &&
+            RedisModule_ModuleTypeGetType(key) != HelloType)
+        {
+            return RedisModule_ReplyWithError(ctx,REDISMODULE_ERRORMSG_WRONGTYPE);
+        }
+        */
+        //raw::key_type(key)
+
+        Ok(())
+    }
 }
 
 impl Drop for RedisKeyWritable {

src/raw.rs

@@ -6,6 +6,10 @@ use std::ffi::CString;
 use std::os::raw::{c_char, c_int, c_long, c_longlong};
 
 extern crate libc;
+extern crate enum_primitive_derive;
+extern crate num_traits;
+
+use num_traits::{FromPrimitive, ToPrimitive};
 
 use libc::size_t;
 
@@ -18,64 +22,53 @@ bitflags! {
     }
 }
 
+#[derive(Primitive)]
+pub enum KeyType {
+    Empty = REDISMODULE_KEYTYPE_EMPTY as isize,
+    String = REDISMODULE_KEYTYPE_STRING as isize,
+    List = REDISMODULE_KEYTYPE_LIST as isize,
+    Hash = REDISMODULE_KEYTYPE_HASH as isize,
+    Set = REDISMODULE_KEYTYPE_SET as isize,
+    ZSet = REDISMODULE_KEYTYPE_ZSET as isize,
+    Module = REDISMODULE_KEYTYPE_MODULE as isize,
+}
+
+impl From<c_int> for KeyType {
+    fn from(v: c_int) -> Self { KeyType::from_i32(v).unwrap() }
+}
 
-#[derive(Debug, PartialEq)]
-#[repr(i32)]
+// This hack is required since derive(Primitive) requires all values to have the same type,
+// and REDISMODULE_REPLY_UNKNOWN is i32 while the rest are u32.
+// Casting to isize inside the enum definition breaks the derive(Primitive) macro.
+const REDISMODULE_REPLY_UNKNOWN_ISIZE: isize = REDISMODULE_REPLY_UNKNOWN as isize;
+const REDISMODULE_REPLY_STRING_ISIZE: isize = REDISMODULE_REPLY_STRING as isize;
+const REDISMODULE_REPLY_ERROR_ISIZE: isize  = REDISMODULE_REPLY_ERROR as isize;
+const REDISMODULE_REPLY_INTEGER_ISIZE: isize  = REDISMODULE_REPLY_INTEGER as isize;
+const REDISMODULE_REPLY_ARRAY_ISIZE: isize  = REDISMODULE_REPLY_ARRAY as isize;
+const REDISMODULE_REPLY_NULL_ISIZE: isize  = REDISMODULE_REPLY_NULL as isize;
+
+#[derive(Primitive, Debug, PartialEq)]
 pub enum ReplyType {
-    Unknown = REDISMODULE_REPLY_UNKNOWN as i32,
-    String = REDISMODULE_REPLY_STRING as i32,
-    Error = REDISMODULE_REPLY_ERROR as i32,
-    Integer = REDISMODULE_REPLY_INTEGER as i32,
-    Array = REDISMODULE_REPLY_ARRAY as i32,
-    Nil = REDISMODULE_REPLY_NULL as i32,
-}
-
-// Tools that can automate this:
-// https://crates.io/crates/enum_primitive
-// https://crates.io/crates/num_enum
-// https://crates.io/crates/enum-primitive-derive
-
-impl From<i32> for ReplyType {
-    fn from(v: i32) -> Self {
-        use crate::raw::ReplyType::*;
-
-        // TODO: Is there a way to do this with a `match`? We have different types of constants here.
-        if v == Unknown as i32 {
-            Unknown
-        } else if v == String as i32 {
-            String
-        } else if v == Error as i32 {
-            Error
-        } else if v == Integer as i32 {
-            Integer
-        } else if v == Array as i32 {
-            Array
-        } else if v == Nil as i32 {
-            Nil
-        } else {
-            panic!("Received unexpected reply type from Redis: {}", v)
-        }
-    }
+    Unknown = REDISMODULE_REPLY_UNKNOWN_ISIZE,
+    String = REDISMODULE_REPLY_STRING_ISIZE,
+    Error = REDISMODULE_REPLY_ERROR_ISIZE,
+    Integer = REDISMODULE_REPLY_INTEGER_ISIZE,
+    Array = REDISMODULE_REPLY_ARRAY_ISIZE,
+    Nil = REDISMODULE_REPLY_NULL_ISIZE,
 }
 
-#[derive(Clone, Copy, Debug, PartialEq)]
-#[repr(i32)]
+impl From<c_int> for ReplyType {
+    fn from(v: c_int) -> Self { ReplyType::from_i32(v).unwrap() }
+}
+
+#[derive(Primitive, Debug, PartialEq)]
 pub enum Status {
-    Ok = REDISMODULE_OK as i32,
-    Err = REDISMODULE_ERR as i32,
+    Ok = REDISMODULE_OK as isize,
+    Err = REDISMODULE_ERR as isize,
 }
 
 impl From<c_int> for Status {
-    fn from(v: c_int) -> Self {
-        // TODO: Is there a way to do this with a `match`? We have different types of constants here.
-        if v == REDISMODULE_OK as c_int {
-            Status::Ok
-        } else if v == REDISMODULE_ERR as c_int {
-            Status::Err
-        } else {
-            panic!("Received unexpected status from Redis: {}", v)
-        }
-    }
+    fn from(v: c_int) -> Self { Status::from_i32(v).unwrap() }
 }
 
 impl From<Status> for c_int {
@@ -93,6 +86,47 @@ impl From<Status> for Result<(), &str> {
     }
 }
 
+
+#[derive(Debug)]
+pub enum CommandFlag {
+    Write,
+    Readonly,
+    Denyoom,
+    Admin,
+    Pubsub,
+    Noscript,
+    Random,
+    SortForScript,
+    Loading,
+    Stale,
+    SkipMonitor,
+    Asking,
+    Fast,
+    Movablekeys,
+}
+
+
+fn command_flag_repr(flag: &CommandFlag) -> &'static str {
+    use crate::raw::CommandFlag::*;
+    match flag {
+        Write => "write",
+        Readonly => "readonly",
+        Denyoom => "denyoom",
+        Admin => "admin",
+        Pubsub => "pubsub",
+        Noscript => "noscript",
+        Random => "random",
+        SortForScript => "sort_for_script",
+        Loading => "loading",
+        Stale => "stale",
+        SkipMonitor => "skip_monitor",
+        Asking => "asking",
+        Fast => "fast",
+        Movablekeys => "movablekeys",
+    }
+}
+
+
 pub type CommandFunc = extern "C" fn(
     ctx: *mut RedisModuleCtx,
     argv: *mut *mut RedisModuleString,
@@ -120,8 +154,8 @@ pub fn create_command(
             firstkey,
             lastkey,
             keystep,
-        ).into()
-    };
+        )
+    }.into();
 
     status.into()
 }
@@ -160,9 +194,7 @@ extern "C" {
 
 ///////////////////////////////////////////////////////////////
 
-
 // Helper functions for the raw bindings.
-// Taken from redis-cell.
 
 pub fn call_reply_type(reply: *mut RedisModuleCallReply) -> ReplyType {
     unsafe {
@@ -270,3 +302,7 @@ pub fn string_ptr_len(str: *mut RedisModuleString, len: *mut size_t) -> *const c
 pub fn string_set(key: *mut RedisModuleKey, str: *mut RedisModuleString) -> Status {
     unsafe { RedisModule_StringSet.unwrap()(key, str).into() }
 }
+
+pub fn key_type(key: *mut RedisModuleKey) -> KeyType {
+    unsafe { RedisModule_KeyType.unwrap()(key) }.into()
+}

src/types.rs

@@ -1,41 +1,30 @@
-use core::ptr;
+use std::ptr;
+use std::cell::RefCell;
 use std::ffi::CString;
 use std::os::raw::{c_int, c_void};
 
 use crate::raw;
 
 pub struct RedisModuleType {
-    raw_type: *mut raw::RedisModuleType,
+    raw_type: RefCell<*mut raw::RedisModuleType>,
 }
 
 // We want to be able to create static instances of this type,
 // which means we need to implement Sync.
 unsafe impl Sync for RedisModuleType {}
 
-pub fn redis_log(
-    ctx: *mut raw::RedisModuleCtx,
-    msg: &str,
-) {
-    let level = CString::new("notice").unwrap(); // FIXME reuse this
-    let msg = CString::new(msg).unwrap();
-    unsafe {
-        raw::RedisModule_Log.unwrap()(ctx, level.as_ptr(), msg.as_ptr());
-    }
-}
-
 impl RedisModuleType {
     pub const fn new() -> Self {
         RedisModuleType {
-            raw_type: ptr::null_mut(),
+            raw_type: RefCell::new(ptr::null_mut()),
         }
     }
 
     pub fn create_data_type(
-        &mut self,
+        &self,
         ctx: *mut raw::RedisModuleCtx,
         name: &str,
     ) -> Result<(), &str> {
-
         if name.len() != 9 {
             let msg = "Redis requires the length of native type names to be exactly 9 characters";
             redis_log(ctx, format!("{}, name is: '{}'", msg, name).as_str());
@@ -71,7 +60,9 @@ impl RedisModuleType {
             return Err("Error: created data type is null");
         }
 
-        self.raw_type = redis_type;
+        *self.raw_type.borrow_mut() = redis_type;
+
+        redis_log(ctx, format!("Created new data type '{}'", name).as_str());
 
         Ok(())
     }
@@ -116,3 +107,14 @@ pub unsafe extern "C" fn MyTypeFree(
 //    eprintln!("MyTypeFree");
 }
 
+pub fn redis_log(
+    ctx: *mut raw::RedisModuleCtx,
+    msg: &str,
+) {
+    let level = CString::new("notice").unwrap(); // FIXME reuse this
+    let msg = CString::new(msg).unwrap();
+    unsafe {
+        raw::RedisModule_Log.unwrap()(ctx, level.as_ptr(), msg.as_ptr());
+    }
+}
+