diff --git a/examples/configuration.rs b/examples/configuration.rs index c143143..1174012 100644 --- a/examples/configuration.rs +++ b/examples/configuration.rs @@ -7,11 +7,12 @@ use lazy_static::lazy_static; use valkey_module::alloc::ValkeyAlloc; use valkey_module::{ configuration::{ConfigurationContext, ConfigurationFlags}, - enum_configuration, valkey_module, ConfigurationValue, Context, ValkeyGILGuard, ValkeyResult, - ValkeyString, ValkeyValue, + enum_configuration, valkey_module, ConfigurationValue, Context, ValkeyError, ValkeyGILGuard, + ValkeyResult, ValkeyString, ValkeyValue, }; enum_configuration! { + #[derive(PartialEq)] enum EnumConfiguration { Val1 = 1, Val2 = 2, @@ -21,15 +22,21 @@ enum_configuration! { lazy_static! { static ref NUM_OF_CONFIGURATION_CHANGES: ValkeyGILGuard = ValkeyGILGuard::default(); static ref CONFIGURATION_I64: ValkeyGILGuard = ValkeyGILGuard::default(); + static ref CONFIGURATION_REJECT_I64: ValkeyGILGuard = ValkeyGILGuard::default(); static ref CONFIGURATION_ATOMIC_I64: AtomicI64 = AtomicI64::new(1); static ref CONFIGURATION_VALKEY_STRING: ValkeyGILGuard = ValkeyGILGuard::new(ValkeyString::create(None, "default")); + static ref CONFIGURATION_REJECT_VALKEY_STRING: ValkeyGILGuard = + ValkeyGILGuard::new(ValkeyString::create(None, "default")); static ref CONFIGURATION_STRING: ValkeyGILGuard = ValkeyGILGuard::new("default".into()); static ref CONFIGURATION_MUTEX_STRING: Mutex = Mutex::new("default".into()); static ref CONFIGURATION_ATOMIC_BOOL: AtomicBool = AtomicBool::default(); static ref CONFIGURATION_BOOL: ValkeyGILGuard = ValkeyGILGuard::default(); + static ref CONFIGURATION_REJECT_BOOL: ValkeyGILGuard = ValkeyGILGuard::default(); static ref CONFIGURATION_ENUM: ValkeyGILGuard = ValkeyGILGuard::new(EnumConfiguration::Val1); + static ref CONFIGURATION_REJECT_ENUM: ValkeyGILGuard = + ValkeyGILGuard::new(EnumConfiguration::Val1); static ref CONFIGURATION_MUTEX_ENUM: Mutex = Mutex::new(EnumConfiguration::Val1); } @@ -43,6 +50,53 @@ fn on_configuration_changed>( *val += 1 } +// Custom on_set handlers to add validation and conditionally +// reject upon config change. +fn on_string_config_set>( + config_ctx: &ConfigurationContext, + _name: &str, + val: &'static T, +) -> Result<(), ValkeyError> { + let v = val.get(config_ctx); + if v.to_string_lossy().contains("rejectvalue") { + return Err(ValkeyError::Str("Rejected from custom string validation.")); + } + Ok(()) +} +fn on_i64_config_set>( + config_ctx: &ConfigurationContext, + _name: &str, + val: &'static T, +) -> Result<(), ValkeyError> { + let v = val.get(config_ctx); + if v == 123 { + return Err(ValkeyError::Str("Rejected from custom i64 validation.")); + } + Ok(()) +} +fn on_bool_config_set>( + config_ctx: &ConfigurationContext, + _name: &str, + val: &'static T, +) -> Result<(), ValkeyError> { + let v = val.get(config_ctx); + if v == false { + return Err(ValkeyError::Str("Rejected from custom bool validation.")); + } + Ok(()) +} +fn on_enum_config_set>( + config_ctx: &ConfigurationContext, + _name: &str, + val: &'static T, +) -> Result<(), ValkeyError> { + let v = val.get(config_ctx); + if v == EnumConfiguration::Val2 { + return Err(ValkeyError::Str("Rejected from custom enum validation.")); + } + Ok(()) +} + fn num_changes(ctx: &Context, _: Vec) -> ValkeyResult { let val = NUM_OF_CONFIGURATION_CHANGES.lock(ctx); Ok(ValkeyValue::Integer(*val)) @@ -61,19 +115,23 @@ valkey_module! { configurations: [ i64: [ ["i64", &*CONFIGURATION_I64, 10, 0, 1000, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed))], + ["reject_i64", &*CONFIGURATION_REJECT_I64, 10, 0, 1000, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed)), Some(Box::new(on_i64_config_set::>))], ["atomic_i64", &*CONFIGURATION_ATOMIC_I64, 10, 0, 1000, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed))], ], string: [ ["valkey_string", &*CONFIGURATION_VALKEY_STRING, "default", ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed))], + ["reject_valkey_string", &*CONFIGURATION_REJECT_VALKEY_STRING, "default", ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed)), Some(Box::new(on_string_config_set::>))], ["string", &*CONFIGURATION_STRING, "default", ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed::))], ["mutex_string", &*CONFIGURATION_MUTEX_STRING, "default", ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed::))], ], bool: [ ["atomic_bool", &*CONFIGURATION_ATOMIC_BOOL, true, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed))], ["bool", &*CONFIGURATION_BOOL, true, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed))], + ["reject_bool", &*CONFIGURATION_REJECT_BOOL, true, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed)), Some(Box::new(on_bool_config_set::>))], ], enum: [ ["enum", &*CONFIGURATION_ENUM, EnumConfiguration::Val1, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed))], + ["reject_enum", &*CONFIGURATION_REJECT_ENUM, EnumConfiguration::Val1, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed)), Some(Box::new(on_enum_config_set::>))], ["enum_mutex", &*CONFIGURATION_MUTEX_ENUM, EnumConfiguration::Val1, ConfigurationFlags::DEFAULT, Some(Box::new(on_configuration_changed))], ], module_args_as_configuration: true, diff --git a/src/configuration.rs b/src/configuration.rs index fcd3010..65e862c 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -175,9 +175,13 @@ impl ConfigurationValue for AtomicBool { type OnUpdatedCallback = Box; +type OnSetCallback = + Box Result<(), ValkeyError>>; + struct ConfigrationPrivateData + 'static> { variable: &'static T, on_changed: Option>, + on_set: Option>, phantom: PhantomData, } @@ -191,12 +195,24 @@ impl + 'static> ConfigrationPrivateData { return raw::REDISMODULE_ERR as i32; } let c_str_name = unsafe { CStr::from_ptr(name) }; + if let Some(v) = self.on_set.as_ref() { + let result = v( + &configuration_ctx, + c_str_name.to_str().unwrap(), + self.variable, + ); + if let Err(e) = result { + let error_msg = ValkeyString::create(None, e.to_string().as_str()); + unsafe { *err = error_msg.take() }; + return raw::REDISMODULE_ERR as i32; + } + } if let Some(v) = self.on_changed.as_ref() { v( &configuration_ctx, c_str_name.to_str().unwrap(), self.variable, - ) + ); } raw::REDISMODULE_OK as i32 } @@ -233,11 +249,13 @@ pub fn register_i64_configuration>( max: i64, flags: ConfigurationFlags, on_changed: Option>, + on_set: Option>, ) { let name = CString::new(name).unwrap(); let config_private_data = ConfigrationPrivateData { variable, on_changed, + on_set, phantom: PhantomData::, }; unsafe { @@ -304,12 +322,14 @@ pub fn register_string_configuration>( default: &str, flags: ConfigurationFlags, on_changed: Option>, + on_set: Option>, ) { let name = CString::new(name).unwrap(); let default = CString::new(default).unwrap(); let config_private_data = ConfigrationPrivateData { variable, on_changed, + on_set, phantom: PhantomData::, }; unsafe { @@ -359,11 +379,13 @@ pub fn register_bool_configuration>( default: bool, flags: ConfigurationFlags, on_changed: Option>, + on_set: Option>, ) { let name = CString::new(name).unwrap(); let config_private_data = ConfigrationPrivateData { variable, on_changed, + on_set, phantom: PhantomData::, }; unsafe { @@ -427,6 +449,7 @@ pub fn register_enum_configuration>, + on_set: Option>, ) { let name = CString::new(name).unwrap(); let (names, vals) = default.get_options(); @@ -438,6 +461,7 @@ pub fn register_enum_configuration, }; unsafe { diff --git a/src/macros.rs b/src/macros.rs index 0ed9567..c9a3621 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -148,28 +148,28 @@ macro_rules! valkey_module { $i64_min:expr, $i64_max:expr, $i64_flags_options:expr, - $i64_on_changed:expr + $i64_on_changed:expr $(, $i64_on_set:expr)? ]),* $(,)*],)? $(string:[$([ $string_configuration_name:expr, $string_configuration_val:expr, $string_default:expr, $string_flags_options:expr, - $string_on_changed:expr + $string_on_changed:expr $(, $string_on_set:expr)? ]),* $(,)*],)? $(bool:[$([ $bool_configuration_name:expr, $bool_configuration_val:expr, $bool_default:expr, $bool_flags_options:expr, - $bool_on_changed:expr + $bool_on_changed:expr $(, $bool_on_set:expr)? ]),* $(,)*],)? $(enum:[$([ $enum_configuration_name:expr, $enum_configuration_val:expr, $enum_default:expr, $enum_flags_options:expr, - $enum_on_changed:expr + $enum_on_changed:expr $(, $enum_on_set:expr)? ]),* $(,)*],)? $(module_args_as_configuration:$use_module_args:expr,)? $(module_config_get:$module_config_get_command:expr,)? @@ -290,7 +290,14 @@ macro_rules! valkey_module { } else { $i64_default }; - register_i64_configuration(&context, $i64_configuration_name, $i64_configuration_val, default, $i64_min, $i64_max, $i64_flags_options, $i64_on_changed); + let mut use_fallback = true; + $( + use_fallback = false; + register_i64_configuration(&context, $i64_configuration_name, $i64_configuration_val, default, $i64_min, $i64_max, $i64_flags_options, $i64_on_changed, $i64_on_set); + )? + if (use_fallback) { + register_i64_configuration(&context, $i64_configuration_name, $i64_configuration_val, default, $i64_min, $i64_max, $i64_flags_options, $i64_on_changed, None); + } )* )? $( @@ -306,7 +313,14 @@ macro_rules! valkey_module { } else { $string_default }; - register_string_configuration(&context, $string_configuration_name, $string_configuration_val, default, $string_flags_options, $string_on_changed); + let mut use_fallback = true; + $( + use_fallback = false; + register_string_configuration(&context, $string_configuration_name, $string_configuration_val, default, $string_flags_options, $string_on_changed, $string_on_set); + )? + if (use_fallback) { + register_string_configuration(&context, $string_configuration_name, $string_configuration_val, default, $string_flags_options, $string_on_changed, None); + } )* )? $( @@ -322,7 +336,14 @@ macro_rules! valkey_module { } else { $bool_default }; - register_bool_configuration(&context, $bool_configuration_name, $bool_configuration_val, default, $bool_flags_options, $bool_on_changed); + let mut use_fallback = true; + $( + use_fallback = false; + register_bool_configuration(&context, $bool_configuration_name, $bool_configuration_val, default, $bool_flags_options, $bool_on_changed, $bool_on_set); + )? + if (use_fallback) { + register_bool_configuration(&context, $bool_configuration_name, $bool_configuration_val, default, $bool_flags_options, $bool_on_changed, None); + } )* )? $( @@ -338,7 +359,14 @@ macro_rules! valkey_module { } else { $enum_default }; - register_enum_configuration(&context, $enum_configuration_name, $enum_configuration_val, default, $enum_flags_options, $enum_on_changed); + let mut use_fallback = true; + $( + use_fallback = false; + register_enum_configuration(&context, $enum_configuration_name, $enum_configuration_val, default.clone(), $enum_flags_options, $enum_on_changed, $enum_on_set); + )? + if (use_fallback) { + register_enum_configuration(&context, $enum_configuration_name, $enum_configuration_val, default.clone(), $enum_flags_options, $enum_on_changed, None); + } )* )? raw::RedisModule_LoadConfigs.unwrap()(ctx); diff --git a/tests/integration.rs b/tests/integration.rs index e4d7616..cf8f572 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -479,7 +479,7 @@ fn test_configuration() -> Result<()> { let res: String = redis::cmd("config") .arg(&["set", config, val]) .query(&mut con) - .with_context(|| "failed to run config set")?; + .map_err(|e| anyhow::anyhow!("Failed to run config set: {}", e))?; assert_eq!(res, "OK"); Ok(()) }; @@ -520,11 +520,55 @@ fn test_configuration() -> Result<()> { config_set("configuration.enum_mutex", "Val2")?; assert_eq!(config_get("configuration.enum_mutex")?, "Val2"); + // Validate that configs can be rejected + let value = config_set("configuration.reject_valkey_string", "rejectvalue"); + assert!(value + .unwrap_err() + .to_string() + .contains("Rejected from custom string validation")); + let value = config_set("configuration.reject_i64", "123"); + assert!(value + .unwrap_err() + .to_string() + .contains("Rejected from custom i64 validation")); + let value = config_set("configuration.reject_bool", "no"); + assert!(value + .unwrap_err() + .to_string() + .contains("Rejected from custom bool validation")); + let value = config_set("configuration.reject_enum", "Val2"); + assert!(value + .unwrap_err() + .to_string() + .contains("Rejected from custom enum validation")); + + let mut con = get_valkey_connection(port).with_context(|| FAILED_TO_CONNECT_TO_SERVER)?; + let res: i64 = redis::cmd("configuration.num_changes") + .query(&mut con) + .with_context(|| "failed to run configuration.num_changes")?; + assert_eq!(res, 26); // the first configuration initialisation is counted as well, so we will get 22 changes. + + // Validate that configs with logic to reject values can also succeed + assert_eq!(config_get("configuration.reject_valkey_string")?, "default"); + config_set("configuration.reject_valkey_string", "validvalue")?; + assert_eq!( + config_get("configuration.reject_valkey_string")?, + "validvalue" + ); + assert_eq!(config_get("configuration.reject_i64")?, "10"); + config_set("configuration.reject_i64", "11")?; + assert_eq!(config_get("configuration.reject_i64")?, "11"); + assert_eq!(config_get("configuration.reject_bool")?, "yes"); + config_set("configuration.reject_bool", "yes")?; + assert_eq!(config_get("configuration.reject_bool")?, "yes"); + assert_eq!(config_get("configuration.reject_enum")?, "Val1"); + config_set("configuration.reject_enum", "Val1")?; + assert_eq!(config_get("configuration.reject_enum")?, "Val1"); let mut con = get_valkey_connection(port).with_context(|| FAILED_TO_CONNECT_TO_SERVER)?; let res: i64 = redis::cmd("configuration.num_changes") .query(&mut con) .with_context(|| "failed to run configuration.num_changes")?; - assert_eq!(res, 18); // the first configuration initialisation is counted as well, so we will get 18 changes. + assert_eq!(res, 28); Ok(()) }