From c32a19e242668538dd09b1b179aca0c4de4d1059 Mon Sep 17 00:00:00 2001 From: Georgy Moshkin <gmoshkin@picodata.io> Date: Thu, 15 Aug 2024 19:21:19 +0300 Subject: [PATCH] fix: we use "config" instead of "cfg" Closes #653 --- picoplugin/src/plugin/interface.rs | 50 +++++++++++++------------- src/plugin/manager.rs | 5 +-- test/int/test_plugin.py | 8 ++--- test/testplug/src/lib.rs | 57 +++++++++++++++--------------- 4 files changed, 60 insertions(+), 60 deletions(-) diff --git a/picoplugin/src/plugin/interface.rs b/picoplugin/src/plugin/interface.rs index 5fa4ef24ff..4424935544 100644 --- a/picoplugin/src/plugin/interface.rs +++ b/picoplugin/src/plugin/interface.rs @@ -119,7 +119,7 @@ pub type CallbackResult<T> = Result<T, ErrorBox>; /// Service trait. Implement it in your code to create a service. pub trait Service { /// Use this associated type to define configuration of your service. - type CFG: DeserializeOwned; + type Config: DeserializeOwned; /// Called before new configration is loaded. /// @@ -127,9 +127,9 @@ pub trait Service { /// /// # Arguments /// - /// * `cfg`: target configuration - fn on_cfg_validate(&self, cfg: Self::CFG) -> CallbackResult<()> { - _ = cfg; + /// * `config`: target configuration + fn on_config_validate(&self, config: Self::Config) -> CallbackResult<()> { + _ = config; Ok(()) } @@ -148,17 +148,17 @@ pub trait Service { /// # Arguments /// /// * `ctx`: instance context - /// * `new_cfg`: new configuration - /// * `old_cfg`: previous defined configuration + /// * `new_config`: new configuration + /// * `old_config`: previous defined configuration fn on_config_change( &mut self, ctx: &PicoContext, - new_cfg: Self::CFG, - old_cfg: Self::CFG, + new_config: Self::Config, + old_config: Self::Config, ) -> CallbackResult<()> { _ = ctx; - _ = new_cfg; - _ = old_cfg; + _ = new_config; + _ = old_config; Ok(()) } @@ -170,10 +170,10 @@ pub trait Service { /// # Arguments /// /// * `context`: instance context - /// * `cfg`: initial configuration - fn on_start(&mut self, context: &PicoContext, cfg: Self::CFG) -> CallbackResult<()> { + /// * `config`: initial configuration + fn on_start(&mut self, context: &PicoContext, config: Self::Config) -> CallbackResult<()> { _ = context; - _ = cfg; + _ = config; Ok(()) } @@ -225,7 +225,7 @@ pub trait Service { /// Define interface like [`Service`] trait but using safe types from [`abi_stable`] crate. #[sabi_trait] pub trait ServiceStable { - fn on_cfg_validate(&self, configuration: RSlice<u8>) -> RResult<(), ()>; + fn on_config_validate(&self, configuration: RSlice<u8>) -> RResult<(), ()>; fn on_health_check(&self, context: &PicoContext) -> RResult<(), ()>; fn on_start(&mut self, context: &PicoContext, configuration: RSlice<u8>) -> RResult<(), ()>; fn on_stop(&mut self, context: &PicoContext) -> RResult<(), ()>; @@ -233,18 +233,18 @@ pub trait ServiceStable { fn on_config_change( &mut self, ctx: &PicoContext, - new_cfg: RSlice<u8>, - old_cfg: RSlice<u8>, + new_config: RSlice<u8>, + old_config: RSlice<u8>, ) -> RResult<(), ()>; } /// Implementation of [`ServiceStable`] pub struct ServiceProxy<C: DeserializeOwned> { - service: Box<dyn Service<CFG = C>>, + service: Box<dyn Service<Config = C>>, } impl<C: DeserializeOwned> ServiceProxy<C> { - pub fn from_service(service: Box<dyn Service<CFG = C>>) -> Self { + pub fn from_service(service: Box<dyn Service<Config = C>>) -> Self { Self { service } } } @@ -273,9 +273,9 @@ macro_rules! rtry { } impl<C: DeserializeOwned> ServiceStable for ServiceProxy<C> { - fn on_cfg_validate(&self, configuration: RSlice<u8>) -> RResult<(), ()> { + fn on_config_validate(&self, configuration: RSlice<u8>) -> RResult<(), ()> { let configuration: C = rtry!(rmp_serde::from_slice(configuration.as_slice())); - let res = self.service.on_cfg_validate(configuration); + let res = self.service.on_config_validate(configuration); match res { Ok(_) => ROk(()), Err(e) => error_into_tt_error(e), @@ -314,13 +314,13 @@ impl<C: DeserializeOwned> ServiceStable for ServiceProxy<C> { fn on_config_change( &mut self, ctx: &PicoContext, - new_cfg: RSlice<u8>, - old_cfg: RSlice<u8>, + new_config: RSlice<u8>, + old_config: RSlice<u8>, ) -> RResult<(), ()> { - let new_cfg: C = rtry!(rmp_serde::from_slice(new_cfg.as_slice())); - let old_cfg: C = rtry!(rmp_serde::from_slice(old_cfg.as_slice())); + let new_config: C = rtry!(rmp_serde::from_slice(new_config.as_slice())); + let old_config: C = rtry!(rmp_serde::from_slice(old_config.as_slice())); - let res = self.service.on_config_change(ctx, new_cfg, old_cfg); + let res = self.service.on_config_change(ctx, new_config, old_config); match res { Ok(_) => ROk(()), Err(e) => error_into_tt_error(e), diff --git a/src/plugin/manager.rs b/src/plugin/manager.rs index cc598d3b62..9ebf45e400 100644 --- a/src/plugin/manager.rs +++ b/src/plugin/manager.rs @@ -715,9 +715,10 @@ impl PluginManager { let service = service.lock(); if service.name == service_name { #[rustfmt::skip] - tlog!(Debug, "calling {}.{}:{}.on_cfg_validate", service.plugin_name, service.name, service.version); + tlog!(Debug, "calling {}.{}:{}.on_config_validate", service.plugin_name, service.name, service.version); - return if let RErr(_) = service.inner.on_cfg_validate(RSlice::from(new_cfg_raw)) { + return if let RErr(_) = service.inner.on_config_validate(RSlice::from(new_cfg_raw)) + { let error = BoxError::last(); Err(PluginError::Callback( PluginCallbackError::InvalidConfiguration(error), diff --git a/test/int/test_plugin.py b/test/int/test_plugin.py index 12e4d3c301..a2b60cde9c 100644 --- a/test/int/test_plugin.py +++ b/test/int/test_plugin.py @@ -1032,7 +1032,7 @@ def test_config_validation(cluster: Cluster): plugin_ref = plugin_ref.install(True).enable(True) plugin_ref.assert_synced() - plugin_ref.inject_error("testservice_1", "on_cfg_validate", "test error", i1) + plugin_ref.inject_error("testservice_1", "on_config_validate", "test error", i1) with pytest.raises( ReturnError, match="New configuration validation error:.* test error" ): @@ -1115,7 +1115,7 @@ def test_error_on_config_update(cluster: Cluster): plugin_ref.assert_config("testservice_1", _DEFAULT_CFG, i1, i2) - plugin_ref.inject_error("testservice_1", "on_cfg_change", "test error", i1) + plugin_ref.inject_error("testservice_1", "on_config_change", "test error", i1) i1.eval( "pico.update_plugin_config('testplug', '0.1.0', 'testservice_1', {foo = " @@ -1154,7 +1154,7 @@ def test_instance_service_poison_and_healthy_then(cluster: Cluster): plugin_ref.assert_synced() plugin_ref.assert_config("testservice_1", _DEFAULT_CFG, i1, i2) - plugin_ref.inject_error("testservice_1", "on_cfg_change", "test error", i1) + plugin_ref.inject_error("testservice_1", "on_config_change", "test error", i1) i1.eval( "pico.update_plugin_config('testplug', '0.1.0', 'testservice_1', {foo = " @@ -1167,7 +1167,7 @@ def test_instance_service_poison_and_healthy_then(cluster: Cluster): lambda: plugin_ref.assert_route_poisoned(i1.instance_id, "testservice_1") ) - plugin_ref.remove_error("testservice_1", "on_cfg_change", i1) + plugin_ref.remove_error("testservice_1", "on_config_change", i1) i1.eval( "pico.update_plugin_config('testplug', '0.1.0', 'testservice_1', {foo = " diff --git a/test/testplug/src/lib.rs b/test/testplug/src/lib.rs index 1725ed71ba..f952467ba0 100644 --- a/test/testplug/src/lib.rs +++ b/test/testplug/src/lib.rs @@ -18,7 +18,6 @@ use picoplugin::{internal, log, system}; use serde::{Deserialize, Serialize}; use std::cell::Cell; use std::fmt::Display; -use std::process::exit; use std::rc::Rc; use std::str::FromStr; use std::sync; @@ -70,12 +69,12 @@ impl ErrInjection { Self::get_err_inj("on_start_sleep_sec", service) } - pub fn err_at_on_cfg_validate(service: &str) -> Option<String> { - Self::get_err_inj("on_cfg_validate", service) + pub fn err_at_on_config_validate(service: &str) -> Option<String> { + Self::get_err_inj("on_config_validate", service) } - pub fn err_at_on_cfg_change(service: &str) -> Option<String> { - Self::get_err_inj("on_cfg_change", service) + pub fn err_at_on_config_change(service: &str) -> Option<String> { + Self::get_err_inj("on_config_change", service) } } @@ -137,13 +136,13 @@ fn inc_callback_calls(service: &str, callback: &str) { lua.exec(&exec_code).unwrap(); } -fn update_plugin_cfg(service: &str, cfg: &Service1Cfg) { +fn update_plugin_config(service: &str, config: &Service1Config) { let lua = tarantool::lua_state(); init_plugin_state_if_need(&lua, service); lua.eval_with::<_, ()>( format!("_G['plugin_state']['{service}']['current_config'] = ...").as_ref(), - cfg, + config, ) .unwrap(); } @@ -161,25 +160,25 @@ fn save_last_seen_context(service: &str, ctx: &PicoContext) { } #[derive(Serialize, Deserialize, Debug, tlua::Push, PartialEq)] -struct Service1Cfg { +struct Service1Config { foo: bool, bar: i32, baz: Vec<String>, } struct Service1 { - cfg: Option<Service1Cfg>, + config: Option<Service1Config>, } impl Service for Service1 { - type CFG = Service1Cfg; + type Config = Service1Config; - fn on_cfg_validate(&self, _configuration: Self::CFG) -> CallbackResult<()> { - if let Some(err_text) = ErrInjection::err_at_on_cfg_validate("testservice_1") { + fn on_config_validate(&self, _configuration: Self::Config) -> CallbackResult<()> { + if let Some(err_text) = ErrInjection::err_at_on_config_validate("testservice_1") { return Err(err_text.into()); } - inc_callback_calls("testservice_1", "on_cfg_validate"); + inc_callback_calls("testservice_1", "on_config_validate"); Ok(()) } @@ -187,25 +186,25 @@ impl Service for Service1 { fn on_config_change( &mut self, ctx: &PicoContext, - new_cfg: Self::CFG, - old_cfg: Self::CFG, + new_config: Self::Config, + old_config: Self::Config, ) -> CallbackResult<()> { if !ErrInjection::get_err_inj::<bool>("assert_config_changed", "testservice_1") .unwrap_or_default() { - assert_ne!(new_cfg, old_cfg); + assert_ne!(new_config, old_config); } - if let Some(err_text) = ErrInjection::err_at_on_cfg_change("testservice_1") { + if let Some(err_text) = ErrInjection::err_at_on_config_change("testservice_1") { return Err(err_text.into()); } save_last_seen_context("testservice_1", ctx); inc_callback_calls("testservice_1", "on_config_change"); - update_plugin_cfg("testservice_1", &new_cfg); + update_plugin_config("testservice_1", &new_config); Ok(()) } - fn on_start(&mut self, ctx: &PicoContext, cfg: Self::CFG) -> CallbackResult<()> { + fn on_start(&mut self, ctx: &PicoContext, config: Self::Config) -> CallbackResult<()> { if let Some(sleep_secs) = ErrInjection::sleep_at_on_start_sec("testservice_1") { fiber::sleep(Duration::from_secs(sleep_secs)); } @@ -216,9 +215,9 @@ impl Service for Service1 { save_last_seen_context("testservice_1", ctx); inc_callback_calls("testservice_1", "on_start"); - update_plugin_cfg("testservice_1", &cfg); + update_plugin_config("testservice_1", &config); - self.cfg = Some(cfg); + self.config = Some(config); Ok(()) } @@ -249,16 +248,16 @@ impl Service for Service1 { impl Service1 { pub fn new() -> Self { - Service1 { cfg: None } + Service1 { config: None } } } struct Service2 {} impl Service for Service2 { - type CFG = (); + type Config = (); - fn on_start(&mut self, ctx: &PicoContext, _: Self::CFG) -> CallbackResult<()> { + fn on_start(&mut self, ctx: &PicoContext, _: Self::Config) -> CallbackResult<()> { if ErrInjection::err_at_on_stop("testservice_2").unwrap_or(false) { return Err("error at `on_stop`".into()); } @@ -304,15 +303,15 @@ fn save_in_lua(service: &str, key: &str, value: impl Display) { } #[derive(Deserialize)] -struct Service3Cfg { +struct Service3Config { test_type: String, } impl Service for Service3 { - type CFG = Service3Cfg; + type Config = Service3Config; - fn on_start(&mut self, ctx: &PicoContext, cfg: Self::CFG) -> CallbackResult<()> { - match cfg.test_type.as_str() { + fn on_start(&mut self, ctx: &PicoContext, config: Self::Config) -> CallbackResult<()> { + match config.test_type.as_str() { "internal" => { let version = internal::picodata_version(); save_in_lua("testservice_3", "version", version); @@ -466,7 +465,7 @@ struct PingResponse { } impl Service for ServiceWithRpcTests { - type CFG = (); + type Config = (); fn on_start(&mut self, context: &PicoContext, _: ()) -> CallbackResult<()> { rpc::RouteBuilder::from(context) -- GitLab