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 3 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
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
9 changes: 4 additions & 5 deletions src/context/call_reply.rs
Original file line number Diff line number Diff line change
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
4 changes: 2 additions & 2 deletions src/context/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)};
unsafe{ drop(CString::from_raw(v.notes as *mut c_char)) };
MeirShpilraien marked this conversation as resolved.
Show resolved Hide resolved
}
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)};
unsafe{ drop(CString::from_raw(v.bs.keyword.keyword as *mut c_char)) };
MeirShpilraien marked this conversation as resolved.
Show resolved Hide resolved
}
}
});
Expand Down
74 changes: 29 additions & 45 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,8 +476,7 @@ impl Context {

#[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() }
raw::reply_with_error(self.ctx, s)
}

pub fn reply_with_key(&self, result: RedisValueKey) -> raw::Status {
Expand Down Expand Up @@ -590,14 +589,14 @@ impl Context {

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
} 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 @@ -781,53 +780,38 @@ 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"))
}

api!(
MeirShpilraien marked this conversation as resolved.
Show resolved Hide resolved
[RedisModule_AddPostNotificationJob],
/// When running inside a key space notification callback, it is dangerous and highly discouraged to perform any write
/// operation. In order to still perform write actions in this scenario, Redis provides this API ([add_post_notification_job])
/// that allows to register a job callback which Redis will call when the following condition holds:
///
/// 1. It is safe to perform any write operation.
/// 2. The job will be called atomically along side the key space notification.
///
/// Notice, one job might trigger key space notifications that will trigger more jobs.
/// This raises a concerns of entering an infinite loops, we consider infinite loops
/// as a logical bug that need to be fixed in the module, an attempt to protect against
/// infinite loops by halting the execution could result in violation of the feature correctness
/// and so Redis will make no attempt to protect the module from infinite loops.
pub fn add_post_notification_job<F: FnOnce(&Context) + 'static>(
&self,
callback: F,
) -> Status {
let callback = Box::into_raw(Box::new(Some(callback)));
unsafe {
RedisModule_AddPostNotificationJob(
self.ctx,
Some(post_notification_job::<F>),
callback as *mut c_void,
Some(post_notification_job_free_callback::<F>),
)
}
.into()
}
);
/// When running inside a key space notification callback, it is dangerous and highly discouraged to perform any write
/// operation. In order to still perform write actions in this scenario, Redis provides this API ([add_post_notification_job])
/// that allows to register a job callback which Redis will call when the following condition holds:
///
/// 1. It is safe to perform any write operation.
/// 2. The job will be called atomically along side the key space notification.
///
/// Notice, one job might trigger key space notifications that will trigger more jobs.
/// This raises a concerns of entering an infinite loops, we consider infinite loops
/// as a logical bug that need to be fixed in the module, an attempt to protect against
/// infinite loops by halting the execution could result in violation of the feature correctness
/// and so Redis will make no attempt to protect the module from infinite loops.
pub fn add_post_notification_job<F: FnOnce(&Context) + 'static>(&self, callback: F) -> Status {
let callback = Box::into_raw(Box::new(Some(callback)));
raw::add_post_notification_job(
self.ctx,
Some(post_notification_job::<F>),
callback as *mut c_void,
Some(post_notification_job_free_callback::<F>),
)
}

api!(
[RedisModule_AvoidReplicaTraffic],
Expand Down Expand Up @@ -873,7 +857,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
8 changes: 2 additions & 6 deletions src/context/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ 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 {
return Err(RedisError::Str(
Expand All @@ -86,10 +85,7 @@ 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 {
return Err(RedisError::Str(
Expand Down
17 changes: 4 additions & 13 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ 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()
raw::key_type(self.key_inner)
}

/// Detects whether the key pointer given to us by Redis is null.
Expand Down Expand Up @@ -358,7 +358,7 @@ 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()
raw::key_type(self.key_inner)
}

#[must_use]
Expand Down Expand Up @@ -402,16 +402,7 @@ impl RedisKeyWritable {
pub fn set_value<T>(&self, redis_type: &RedisType, value: T) -> Result<(), RedisError> {
verify_type(self.key_inner, redis_type)?;
let value = Box::into_raw(Box::new(value)).cast::<c_void>();
let status: raw::Status = unsafe {
raw::RedisModule_ModuleTypeSetValue.unwrap()(
self.key_inner,
*redis_type.raw_type.borrow(),
value,
)
}
.into();

status.into()
raw::module_type_set_value(self.key_inner, *redis_type.raw_type.borrow(), value).into()
}

pub fn trim_stream_by_id(
Expand Down Expand Up @@ -654,7 +645,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 = raw::key_type(key_inner);

if key_type != KeyType::Empty {
// The key exists; check its type
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub use crate::raw::*;
pub use crate::redismodule::*;
use backtrace::Backtrace;
use context::server_events::INFO_COMMAND_HANDLER_LIST;
pub use redis_module_macros::*;

/// The detached Redis module context (the context of this module). It
/// is only set to a proper value after the module is initialised via the
Expand Down
Loading
Loading