Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update dependencies and refactor the FFI calls #375

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -119,16 +119,17 @@ crate-type = ["cdylib"]
[dependencies]
bitflags = "2"
libc = "0.2"
enum-primitive-derive = "^0.1"
num-traits = "^0.2"
enum-primitive-derive = "0.3"
num-traits = "0.2"
regex = "1"
strum_macros = "0.24"
strum_macros = "0.25"
backtrace = "0.3"
linkme = "0.3"
serde = { version = "1", features = ["derive"] }
nix = "0.26"
nix = "0.27"
cfg-if = "1"
redis-module-macros-internals = { path = "./redismodule-rs-macros-internals" }
redis-module-macros = { path = "./redismodule-rs-macros"}
log = "0.4"

[dev-dependencies]
Expand All @@ -139,7 +140,7 @@ redis-module-macros = { path = "./redismodule-rs-macros"}
redis-module = { path = "./", default-features = false, features = ["min-redis-compatibility-version-7-2"] }

[build-dependencies]
bindgen = "0.66"
bindgen = "0.69"
cc = "1"

[features]
Expand Down
2 changes: 2 additions & 0 deletions examples/info_handler_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use redis_module::{redis_module, RedisResult};
use redis_module::{InfoContext, Status};
use redis_module_macros::info_command_handler;

// The deprecated methods are allowed since this is just an example.
#[allow(deprecated)]
#[info_command_handler]
fn add_info(ctx: &InfoContext, _for_crash_report: bool) -> RedisResult<()> {
if ctx.add_info_section(Some("info")) == Status::Ok {
Expand Down
3 changes: 1 addition & 2 deletions examples/open_key_with_flags.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use redis_module::{
key::KeyFlags, raw, redis_module, Context, NextArg, RedisError, RedisResult, RedisString,
RedisValue,
key::KeyFlags, redis_module, Context, NextArg, RedisError, RedisResult, RedisString, RedisValue,
};
use redis_module_macros::command;

Expand Down
3 changes: 1 addition & 2 deletions examples/proc_macro_commands.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};

use redis_module::RedisError;
use redis_module::{redis_module, Context, RedisResult, RedisString, RedisValue};
use redis_module_macros::{command, RedisValue};
use redis_module::{command, redis_module, Context, RedisResult, RedisString, RedisValue};

#[derive(RedisValue)]
struct RedisValueDeriveInner {
Expand Down
3 changes: 3 additions & 0 deletions examples/test_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ fn test_helper_err(ctx: &Context, args: Vec<RedisString>) -> RedisResult {
Ok(().into())
}

// This is a function to test that the deprecated API still works fine,
// there is no need to check for the deprecated calls.
#[allow(deprecated)]
fn add_info(ctx: &InfoContext, _for_crash_report: bool) {
if ctx.add_info_section(Some("test_helper")) == Status::Ok {
ctx.add_info_field_str("field", "value");
Expand Down
2 changes: 1 addition & 1 deletion redismodule-rs-macros-internals/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ categories = ["database", "api-bindings"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
syn = { version="1", features = ["full", "extra-traits"]}
syn = { version = "1", features = ["full", "extra-traits"]}
quote = "1"
lazy_static = "1"
proc-macro2 = "1"
Expand Down
6 changes: 3 additions & 3 deletions redismodule-rs-macros/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ categories = ["database", "api-bindings"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
syn = { version="1.0", features = ["full"]}
quote = "1.0"
syn = { version = "1", features = ["full"]}
quote = "1"
proc-macro2 = "1"
serde = { version = "1", features = ["derive"] }
serde_syn = "0.1.0"
serde_syn = "0.1"

[lib]
name = "redis_module_macros"
Expand Down
11 changes: 5 additions & 6 deletions src/context/call_reply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::{

use libc::c_void;

use crate::{deallocate_pointer, raw::*, Context, RedisError, RedisLockIndicator};
use crate::{deallocate_pointer, raw::*, Context, RedisError, RedisLockIndicator, Status};

pub struct StringCallReply<'root> {
reply: NonNull<RedisModuleCallReply>,
Expand Down Expand Up @@ -753,11 +753,10 @@ where
lock_indicator: &LockIndicator,
) -> Status {
let mut callback: *mut C = std::ptr::null_mut();
let res = unsafe {
RedisModule_CallReplyPromiseAbort
.expect("RedisModule_CallReplyPromiseAbort is expected to be available if we got a promise call reply")
(self.reply.as_ptr(), &mut callback as *mut *mut C as *mut *mut c_void)
}.into();
let res = crate::raw::call_reply_promise_abort(
self.reply.as_ptr(),
&mut callback as *mut *mut C as *mut *mut c_void,
);

if !callback.is_null() {
unsafe { deallocate_pointer(callback) };
Expand Down
8 changes: 4 additions & 4 deletions src/context/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ api! {[
0,
0,
)
} == raw::Status::Err as i32
} == Status::Err as i32
{
return Err(RedisError::String(format!(
"Failed register command {}.",
Expand Down Expand Up @@ -444,7 +444,7 @@ api! {[
args: ptr::null_mut(),
};

if unsafe { RedisModule_SetCommandInfo(command, &mut redis_command_info as *mut raw::RedisModuleCommandInfo) } == raw::Status::Err as i32 {
if unsafe { RedisModule_SetCommandInfo(command, &mut redis_command_info as *mut raw::RedisModuleCommandInfo) } == Status::Err as i32 {
return Err(RedisError::String(format!(
"Failed setting info for command {}.",
command_info.name
Expand All @@ -454,12 +454,12 @@ api! {[
// the only CString pointers which are not freed are those of the key_specs, lets free them here.
key_specs.into_iter().for_each(|v|{
if !v.notes.is_null() {
unsafe{CString::from_raw(v.notes as *mut c_char)};
drop(unsafe { CString::from_raw(v.notes as *mut c_char) });
}
if v.begin_search_type == raw::RedisModuleKeySpecBeginSearchType_REDISMODULE_KSPEC_BS_KEYWORD {
let keyword = unsafe{v.bs.keyword.keyword};
if !keyword.is_null() {
unsafe{CString::from_raw(v.bs.keyword.keyword as *mut c_char)};
drop(unsafe { CString::from_raw(v.bs.keyword.keyword as *mut c_char) });
}
}
});
Expand Down
50 changes: 22 additions & 28 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,18 +469,17 @@ impl Context {
}

#[allow(clippy::must_use_candidate)]
pub fn reply_simple_string(&self, s: &str) -> raw::Status {
pub fn reply_simple_string(&self, s: &str) -> Status {
let msg = Self::str_as_legal_resp_string(s);
raw::reply_with_simple_string(self.ctx, msg.as_ptr())
}

#[allow(clippy::must_use_candidate)]
pub fn reply_error_string(&self, s: &str) -> raw::Status {
let msg = Self::str_as_legal_resp_string(s);
unsafe { raw::RedisModule_ReplyWithError.unwrap()(self.ctx, msg.as_ptr()).into() }
pub fn reply_error_string(&self, s: &str) -> Status {
raw::reply_with_error(self.ctx, s)
}

pub fn reply_with_key(&self, result: RedisValueKey) -> raw::Status {
pub fn reply_with_key(&self, result: RedisValueKey) -> Status {
match result {
RedisValueKey::Integer(i) => raw::reply_with_long_long(self.ctx, i),
RedisValueKey::String(s) => {
Expand All @@ -498,7 +497,7 @@ impl Context {
///
/// Will panic if methods used are missing in redismodule.h
#[allow(clippy::must_use_candidate)]
pub fn reply(&self, result: RedisResult) -> raw::Status {
pub fn reply(&self, result: RedisResult) -> Status {
match result {
Ok(RedisValue::Bool(v)) => raw::reply_with_bool(self.ctx, v.into()),
Ok(RedisValue::Integer(v)) => raw::reply_with_long_long(self.ctx, v),
Expand Down Expand Up @@ -541,7 +540,7 @@ impl Context {
self.reply(Ok(elem));
}

raw::Status::Ok
Status::Ok
}

Ok(RedisValue::Map(map)) => {
Expand All @@ -552,7 +551,7 @@ impl Context {
self.reply(Ok(value));
}

raw::Status::Ok
Status::Ok
}

Ok(RedisValue::OrderedMap(map)) => {
Expand All @@ -563,7 +562,7 @@ impl Context {
self.reply(Ok(value));
}

raw::Status::Ok
Status::Ok
}

Ok(RedisValue::Set(set)) => {
Expand All @@ -572,7 +571,7 @@ impl Context {
self.reply_with_key(e);
});

raw::Status::Ok
Status::Ok
}

Ok(RedisValue::OrderedSet(set)) => {
Expand All @@ -581,23 +580,23 @@ impl Context {
self.reply_with_key(e);
});

raw::Status::Ok
Status::Ok
}

Ok(RedisValue::Null) => raw::reply_with_null(self.ctx),

Ok(RedisValue::NoReply) => raw::Status::Ok,
Ok(RedisValue::NoReply) => Status::Ok,

Ok(RedisValue::StaticError(s)) => self.reply_error_string(s),

Err(RedisError::WrongArity) => unsafe {
Err(RedisError::WrongArity) => {
if self.is_keys_position_request() {
// We can't return a result since we don't have a client
raw::Status::Err
Status::Err
} else {
raw::RedisModule_WrongArity.unwrap()(self.ctx).into()
raw::wrong_arity(self.ctx)
}
},
}

Err(RedisError::WrongType) => {
self.reply_error_string(RedisError::WrongType.to_string().as_str())
Expand Down Expand Up @@ -672,7 +671,7 @@ impl Context {
event_type: raw::NotifyEvent,
event: &str,
keyname: &RedisString,
) -> raw::Status {
) -> Status {
unsafe { raw::notify_keyspace_event(self.ctx, event_type, event, keyname) }
}

Expand Down Expand Up @@ -781,18 +780,12 @@ impl Context {
key_name: &RedisString,
permissions: &AclPermissions,
) -> Result<(), RedisError> {
let user = unsafe { raw::RedisModule_GetModuleUserFromUserName.unwrap()(user_name.inner) };
let user = raw::get_module_user_from_user_name(user_name.inner);
if user.is_null() {
return Err(RedisError::Str("User does not exists or disabled"));
}
let acl_permission_result: raw::Status = unsafe {
raw::RedisModule_ACLCheckKeyPermissions.unwrap()(
user,
key_name.inner,
permissions.bits(),
)
}
.into();
let acl_permission_result =
raw::acl_check_key_permissions(user, key_name.inner, permissions.bits());
unsafe { raw::RedisModule_FreeModuleUser.unwrap()(user) };
let acl_permission_result: Result<(), &str> = acl_permission_result.into();
acl_permission_result.map_err(|_e| RedisError::Str("User does not have permissions on key"))
Expand Down Expand Up @@ -825,7 +818,8 @@ impl Context {
Some(post_notification_job_free_callback::<F>),
)
}
.into()
.try_into()
.unwrap()
}
);

Expand Down Expand Up @@ -873,7 +867,7 @@ impl Context {
}

extern "C" fn post_notification_job_free_callback<F: FnOnce(&Context)>(pd: *mut c_void) {
unsafe { Box::from_raw(pd as *mut Option<F>) };
unsafe { drop(Box::from_raw(pd as *mut Option<F>)) };
}

extern "C" fn post_notification_job<F: FnOnce(&Context)>(
Expand Down
4 changes: 2 additions & 2 deletions src/context/thread_safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::ops::{Deref, DerefMut};
use std::ptr;

use crate::context::blocked::BlockedClient;
use crate::{raw, Context, RedisResult};
use crate::{raw, Context, RedisResult, Status};

pub struct RedisGILGuardScope<'ctx, 'mutex, T, G: RedisLockIndicator> {
_context: &'ctx G,
Expand Down Expand Up @@ -158,7 +158,7 @@ impl ThreadSafeContext<BlockedClient> {
/// The Redis modules API does not require locking for `Reply` functions,
/// so we pass through its functionality directly.
#[allow(clippy::must_use_candidate)]
pub fn reply(&self, r: RedisResult) -> raw::Status {
pub fn reply(&self, r: RedisResult) -> Status {
let ctx = Context::new(self.ctx);
ctx.reply(r)
}
Expand Down
14 changes: 5 additions & 9 deletions src/context/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::convert::TryInto;
use std::ffi::c_void;
use std::time::Duration;

use crate::raw;
use crate::raw::RedisModuleTimerID;
use crate::{raw, Status};
use crate::{Context, RedisError};

// We use `repr(C)` since we access the underlying data field directly.
Expand Down Expand Up @@ -60,10 +60,9 @@ impl Context {
pub fn stop_timer<T>(&self, timer_id: RedisModuleTimerID) -> Result<T, RedisError> {
let mut data: *mut c_void = std::ptr::null_mut();

let status: raw::Status =
unsafe { raw::RedisModule_StopTimer.unwrap()(self.ctx, timer_id, &mut data) }.into();
let status = raw::stop_timer(self.ctx, timer_id, &mut data);

if status != raw::Status::Ok {
if status != Status::Ok {
return Err(RedisError::Str(
"RedisModule_StopTimer failed, timer may not exist",
));
Expand All @@ -86,12 +85,9 @@ impl Context {
let mut remaining: u64 = 0;
let mut data: *mut c_void = std::ptr::null_mut();

let status: raw::Status = unsafe {
raw::RedisModule_GetTimerInfo.unwrap()(self.ctx, timer_id, &mut remaining, &mut data)
}
.into();
let status = raw::get_timer_info(self.ctx, timer_id, &mut remaining, &mut data);

if status != raw::Status::Ok {
if status != Status::Ok {
return Err(RedisError::Str(
"RedisModule_GetTimerInfo failed, timer may not exist",
));
Expand Down
Loading
Loading