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 1 commit
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
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
2 changes: 1 addition & 1 deletion 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
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{ drop(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{ drop(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
76 changes: 43 additions & 33 deletions src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,17 +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 {
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 @@ -497,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 @@ -540,7 +540,7 @@ impl Context {
self.reply(Ok(elem));
}

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

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

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

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

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

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

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

Ok(RedisValue::OrderedSet(set)) => {
Expand All @@ -580,19 +580,19 @@ 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) => {
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::wrong_arity(self.ctx)
}
Expand Down Expand Up @@ -671,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 @@ -791,27 +791,37 @@ impl Context {
acl_permission_result.map_err(|_e| RedisError::Str("User does not have permissions on key"))
}

/// 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_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>),
)
}
.try_into()
.unwrap()
}
);

api!(
[RedisModule_AvoidReplicaTraffic],
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
6 changes: 3 additions & 3 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 @@ -62,7 +62,7 @@ impl Context {

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 @@ -87,7 +87,7 @@ impl Context {

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
21 changes: 11 additions & 10 deletions src/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::stream::StreamIterator;
use crate::RedisError;
use crate::RedisResult;
use crate::RedisString;
use crate::Status;
use bitflags::bitflags;

/// `RedisKey` is an abstraction over a Redis key that allows readonly
Expand Down Expand Up @@ -240,12 +241,12 @@ impl RedisKeyWritable {
}

#[allow(clippy::must_use_candidate)]
pub fn hash_set(&self, field: &str, value: RedisString) -> raw::Status {
pub fn hash_set(&self, field: &str, value: RedisString) -> Status {
raw::hash_set(self.key_inner, field, value.inner)
}

#[allow(clippy::must_use_candidate)]
pub fn hash_del(&self, field: &str) -> raw::Status {
pub fn hash_del(&self, field: &str) -> Status {
raw::hash_del(self.key_inner, field)
}

Expand Down Expand Up @@ -273,13 +274,13 @@ impl RedisKeyWritable {

// `list_push_head` inserts the specified element at the head of the list stored at this key.
#[allow(clippy::must_use_candidate)]
pub fn list_push_head(&self, element: RedisString) -> raw::Status {
pub fn list_push_head(&self, element: RedisString) -> Status {
raw::list_push(self.key_inner, raw::Where::ListHead, element.inner)
}

// `list_push_tail` inserts the specified element at the tail of the list stored at this key.
#[allow(clippy::must_use_candidate)]
pub fn list_push_tail(&self, element: RedisString) -> raw::Status {
pub fn list_push_tail(&self, element: RedisString) -> Status {
raw::list_push(self.key_inner, raw::Where::ListTail, element.inner)
}

Expand Down Expand Up @@ -321,19 +322,19 @@ impl RedisKeyWritable {
})?;

match raw::set_expire(self.key_inner, exp_time) {
raw::Status::Ok => REDIS_OK,
Status::Ok => REDIS_OK,

// Error may occur if the key wasn't open for writing or is an
// empty key.
raw::Status::Err => Err(RedisError::Str("Error while setting key expire")),
Status::Err => Err(RedisError::Str("Error while setting key expire")),
}
}

pub fn write(&self, val: &str) -> RedisResult {
let val_str = RedisString::create(NonNull::new(self.ctx), val);
match raw::string_set(self.key_inner, val_str.inner) {
raw::Status::Ok => REDIS_OK,
raw::Status::Err => Err(RedisError::Str("Error while setting key")),
Status::Ok => REDIS_OK,
Status::Err => Err(RedisError::Str("Error while setting key")),
}
}

Expand Down Expand Up @@ -569,7 +570,7 @@ impl<'a> StringDMA<'a> {

pub fn write(&mut self, data: &[u8]) -> Result<&mut Self, RedisError> {
if self.buffer.len() != data.len() {
if raw::Status::Ok == raw::string_truncate(self.key.key_inner, data.len()) {
if Status::Ok == raw::string_truncate(self.key.key_inner, data.len()) {
let mut length: size_t = 0;
let dma = raw::string_dma(self.key.key_inner, &mut length, raw::KeyMode::WRITE);
self.buffer = unsafe { std::slice::from_raw_parts_mut(dma.cast::<u8>(), length) };
Expand All @@ -584,7 +585,7 @@ impl<'a> StringDMA<'a> {
pub fn append(&mut self, data: &[u8]) -> Result<&mut Self, RedisError> {
let current_len = self.buffer.len();
let new_len = current_len + data.len();
if raw::Status::Ok == raw::string_truncate(self.key.key_inner, new_len) {
if Status::Ok == raw::string_truncate(self.key.key_inner, new_len) {
let mut length: size_t = 0;
let dma = raw::string_dma(self.key.key_inner, &mut length, raw::KeyMode::WRITE);
self.buffer = unsafe { std::slice::from_raw_parts_mut(dma.cast::<u8>(), length) };
Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@ pub use crate::context::{
InfoContextBuilderFieldBottomLevelValue, InfoContextBuilderFieldTopLevelValue,
InfoContextFieldBottomLevelData, InfoContextFieldTopLevelData, OneInfoSectionData,
};
pub use crate::raw::*;
mod status;
pub(crate) use crate::raw::*;
pub use crate::redismodule::*;
use backtrace::Backtrace;
use context::server_events::INFO_COMMAND_HANDLER_LIST;
pub use redis_module_macros::*;
pub use status::Status;

/// 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