From f8cf77f3290b2c7a61aaecd8a7d36378898d1eca Mon Sep 17 00:00:00 2001 From: cgettys-microsoft <67080058+cgettys-microsoft@users.noreply.github.com> Date: Fri, 27 Dec 2024 16:23:42 -0800 Subject: [PATCH] [FabricRuntime] Support Package change notification callbacks (#96) End user applications probably want to handle package change notifications. This PR currently tackles only ConfigurationPackage. If you're happy with the approach, I'll do the same exact implementation (edit: in additional PRs) for CodePackage and DataPackage and that will close out #97. --- crates/libs/core/src/client/notification.rs | 2 + .../core/src/runtime/activation_context.rs | 47 ++++++- crates/libs/core/src/runtime/mod.rs | 3 + .../core/src/runtime/package_change/config.rs | 119 ++++++++++++++++++ .../core/src/runtime/package_change/mod.rs | 17 +++ crates/samples/echomain/src/main.rs | 49 ++++++-- crates/samples/no_default_features/src/lib.rs | 24 +++- 7 files changed, 243 insertions(+), 18 deletions(-) create mode 100644 crates/libs/core/src/runtime/package_change/config.rs create mode 100644 crates/libs/core/src/runtime/package_change/mod.rs diff --git a/crates/libs/core/src/client/notification.rs b/crates/libs/core/src/client/notification.rs index 002b73ca..aff9c13e 100644 --- a/crates/libs/core/src/client/notification.rs +++ b/crates/libs/core/src/client/notification.rs @@ -136,6 +136,7 @@ impl PartialOrd for ServiceEndpointsVersion { // Bridge implementation for the notification handler to turn rust code into SF com object. #[windows_core::implement(IFabricServiceNotificationEventHandler)] +#[allow(non_camel_case_types)] // Suppress lint for _Impl struct pub struct ServiceNotificationEventHandlerBridge where T: ServiceNotificationEventHandler, @@ -174,6 +175,7 @@ where /// Lambda implemnentation of ServiceNotificationEventHandler trait. /// This is used in FabricClientBuilder to build function into handler. /// Not exposed to user. +/// This isn't strictly required by the implementation as written. But it leaves open the door to non-lambda implementations in future. pub struct LambdaServiceNotificationHandler where T: Fn(&ServiceNotification) -> crate::Result<()> + 'static, diff --git a/crates/libs/core/src/runtime/activation_context.rs b/crates/libs/core/src/runtime/activation_context.rs index 7f321221..e9138b83 100644 --- a/crates/libs/core/src/runtime/activation_context.rs +++ b/crates/libs/core/src/runtime/activation_context.rs @@ -4,7 +4,9 @@ // ------------------------------------------------------------ use mssf_com::{ - FabricRuntime::IFabricCodePackageActivationContext6, + FabricRuntime::{ + IFabricCodePackageActivationContext6, IFabricConfigurationPackageChangeHandler, + }, FabricTypes::{FABRIC_HEALTH_INFORMATION, FABRIC_HEALTH_REPORT_SEND_OPTIONS}, }; @@ -14,7 +16,16 @@ use crate::{ Error, WString, PCWSTR, }; -use super::config::ConfigurationPackage; +use super::{ + config::ConfigurationPackage, + package_change::{ + config::{ + ConfigurationPackageChangeCallbackHandle, ConfigurationPackageChangeEventHandlerBridge, + LambdaConfigurationPackageEventHandler, + }, + ConfigurationPackageChangeEvent, + }, +}; #[derive(Debug, Clone)] pub struct CodePackageActivationContext { @@ -132,6 +143,38 @@ impl CodePackageActivationContext { pub fn get_com(&self) -> IFabricCodePackageActivationContext6 { self.com_impl.clone() } + + pub fn register_configuration_package_change_handler( + &self, + handler: T, + ) -> crate::Result + where + T: Fn(&ConfigurationPackageChangeEvent) + 'static, + { + let lambda_handler = LambdaConfigurationPackageEventHandler::new(handler); + let bridge = ConfigurationPackageChangeEventHandlerBridge::new(lambda_handler); + let callback: IFabricConfigurationPackageChangeHandler = bridge.into(); + // SAFETY: bridge implements the required COM interface + let raw_handle = unsafe { + self.com_impl + .RegisterConfigurationPackageChangeHandler(&callback) + }?; + // SAFETY: raw_handle is a configuration package change handler id, not some other id. + Ok(unsafe { ConfigurationPackageChangeCallbackHandle::from_com(raw_handle) }) + } + + pub fn unregister_configuration_package_change_handler( + &self, + handle: ConfigurationPackageChangeCallbackHandle, + ) -> crate::Result<()> { + // SAFETY: we assume that only 1 activation context can be + unsafe { + self.com_impl + .UnregisterConfigurationPackageChangeHandler(handle.0) + } + .unwrap(); + Ok(()) + } } impl From for CodePackageActivationContext { diff --git a/crates/libs/core/src/runtime/mod.rs b/crates/libs/core/src/runtime/mod.rs index 4794bb4d..00b20309 100644 --- a/crates/libs/core/src/runtime/mod.rs +++ b/crates/libs/core/src/runtime/mod.rs @@ -17,6 +17,9 @@ pub mod error; pub mod executor; #[cfg(feature = "tokio_async")] pub mod node_context; + +pub mod package_change; + #[cfg(feature = "tokio_async")] pub mod runtime_wrapper; #[cfg(feature = "tokio_async")] diff --git a/crates/libs/core/src/runtime/package_change/config.rs b/crates/libs/core/src/runtime/package_change/config.rs new file mode 100644 index 00000000..6bb7733a --- /dev/null +++ b/crates/libs/core/src/runtime/package_change/config.rs @@ -0,0 +1,119 @@ +// ------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See License.txt in the repo root for license information. +// ------------------------------------------------------------ +//! Handle callbacks for configuration package changes +//! TODO: We probably should also provide a helpful callback to use in conjunction with the config-rs support (so that it processes configuration changes) +use mssf_com::FabricRuntime::{ + IFabricConfigurationPackageChangeHandler, IFabricConfigurationPackageChangeHandler_Impl, +}; + +use crate::runtime::config::ConfigurationPackage; + +use super::ConfigurationPackageChangeEvent; + +/// Rust trait to turn rust code into IFabricConfigurationPackageChangeHandler. +/// Not exposed to user +pub trait ConfigurationPackageChangeEventHandler: 'static { + fn on_change(&self, change: &ConfigurationPackageChangeEvent); +} + +// Bridge implementation for the change handler to turn rust code into SF com object. +#[windows_core::implement(IFabricConfigurationPackageChangeHandler)] +#[allow(non_camel_case_types)] // Suppress lint for _Impl struct +pub struct ConfigurationPackageChangeEventHandlerBridge +where + T: ConfigurationPackageChangeEventHandler, +{ + inner: T, +} + +impl ConfigurationPackageChangeEventHandlerBridge +where + T: ConfigurationPackageChangeEventHandler, +{ + pub fn new(inner: T) -> Self { + Self { inner } + } +} + +impl IFabricConfigurationPackageChangeHandler_Impl + for ConfigurationPackageChangeEventHandlerBridge_Impl +where + T: ConfigurationPackageChangeEventHandler, +{ + fn OnPackageAdded( + &self, + _source: Option<&mssf_com::FabricRuntime::IFabricCodePackageActivationContext>, + configpackage: Option<&mssf_com::FabricRuntime::IFabricConfigurationPackage>, + ) { + let new_package = ConfigurationPackage::from_com(configpackage.unwrap().clone()); + let event = ConfigurationPackageChangeEvent::Addition { new_package }; + self.inner.on_change(&event) + } + + fn OnPackageRemoved( + &self, + _source: Option<&mssf_com::FabricRuntime::IFabricCodePackageActivationContext>, + configpackage: Option<&mssf_com::FabricRuntime::IFabricConfigurationPackage>, + ) { + let previous_package = ConfigurationPackage::from_com(configpackage.unwrap().clone()); + let event = ConfigurationPackageChangeEvent::Removal { previous_package }; + self.inner.on_change(&event) + } + + fn OnPackageModified( + &self, + _source: Option<&mssf_com::FabricRuntime::IFabricCodePackageActivationContext>, + previousconfigpackage: Option<&mssf_com::FabricRuntime::IFabricConfigurationPackage>, + configpackage: Option<&mssf_com::FabricRuntime::IFabricConfigurationPackage>, + ) { + let new_package = ConfigurationPackage::from_com(configpackage.unwrap().clone()); + let previous_package = + ConfigurationPackage::from_com(previousconfigpackage.unwrap().clone()); + let event = ConfigurationPackageChangeEvent::Modification { + previous_package, + new_package, + }; + self.inner.on_change(&event) + } +} + +/// Lambda implementation of ConfigurationPackageChangeEventHandler trait. +/// This is used in FabricClientBuilder to build function into handler. +/// Not exposed to user. +/// Strictly speaking we don't need this layer. But it would allow us to open the door to trait implementations someday +pub(crate) struct LambdaConfigurationPackageEventHandler +where + T: Fn(&ConfigurationPackageChangeEvent), +{ + f: T, +} + +impl LambdaConfigurationPackageEventHandler +where + T: Fn(&ConfigurationPackageChangeEvent) + 'static, +{ + pub fn new(f: T) -> Self { + Self { f } + } +} + +impl ConfigurationPackageChangeEventHandler for LambdaConfigurationPackageEventHandler +where + T: Fn(&ConfigurationPackageChangeEvent) + 'static, +{ + fn on_change(&self, change: &ConfigurationPackageChangeEvent) { + (self.f)(change) + } +} + +pub struct ConfigurationPackageChangeCallbackHandle(pub(crate) i64); + +impl ConfigurationPackageChangeCallbackHandle { + /// # Safety + /// Caller ensures this is a registered callback id + pub const unsafe fn from_com(com: i64) -> Self { + Self(com) + } +} diff --git a/crates/libs/core/src/runtime/package_change/mod.rs b/crates/libs/core/src/runtime/package_change/mod.rs new file mode 100644 index 00000000..5d7aeaa6 --- /dev/null +++ b/crates/libs/core/src/runtime/package_change/mod.rs @@ -0,0 +1,17 @@ +// ------------------------------------------------------------ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License (MIT). See License.txt in the repo root for license information. +// ------------------------------------------------------------ +//! This module supports implementing callbacks when Service Fabric Packages are changed +//! +pub mod config; + +/// The ways a given Service Fabric Package (e.g. ConfigurationPackage or DataPackage) can change +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum PackageChangeEvent { + Addition { new_package: T }, + Removal { previous_package: T }, + Modification { previous_package: T, new_package: T }, +} + +pub type ConfigurationPackageChangeEvent = PackageChangeEvent; diff --git a/crates/samples/echomain/src/main.rs b/crates/samples/echomain/src/main.rs index ba23d619..721375ab 100644 --- a/crates/samples/echomain/src/main.rs +++ b/crates/samples/echomain/src/main.rs @@ -9,8 +9,10 @@ use mssf_core::conf::{Config, FabricConfigSource}; use mssf_core::debug::wait_for_debugger; use mssf_core::error::FabricError; +use mssf_core::runtime::config::ConfigurationPackage; use mssf_core::runtime::executor::{DefaultExecutor, Executor}; use mssf_core::runtime::node_context::NodeContext; +use mssf_core::runtime::package_change::PackageChangeEvent; use mssf_core::runtime::CodePackageActivationContext; use mssf_core::types::{HealthInformation, HealthReportSendOption}; use mssf_core::WString; @@ -84,6 +86,37 @@ fn validate_configs(actctx: &CodePackageActivationContext) { let config = actctx .get_configuration_package(&WString::from("Config")) .unwrap(); + let s = build_config(config); + let val = s.get::("my_config_section.my_string").unwrap(); + info!("entry: {}", val); + // note that the config name lookup is case sensitive for struct fields. + let settings = s.try_deserialize::().unwrap(); + info!("settings: {:?}", settings); + let sect = settings.my_config_section; + assert_eq!(sect.my_string, "Value1"); + assert!(sect.my_bool); + assert_eq!(sect.my_int, 99); + actctx.register_configuration_package_change_handler(|change| { + let (some_package, change_type, validate_new) = match change + { + PackageChangeEvent::Addition { new_package } => (new_package, "Addition", true), + PackageChangeEvent::Removal { previous_package } => (previous_package, "Removal", false), + PackageChangeEvent::Modification { previous_package: _, new_package } => (new_package, "Modification", true), + }; + let changed_package_name = some_package.get_description().name.to_string_lossy(); + let changed_package_str = &changed_package_name; + info!("Received config package change of type {change_type:?} to package {changed_package_str}"); + if validate_new + { + // This is a bit hacky, but if there was a removal, not much point in validating the old package + // In an application that actually uses its config settings, we'd probably put the result of this into a OnceLock> + // or something more complicated, like a ArcSwap or similar + build_config(some_package.clone()); + } + }).unwrap(); +} + +fn build_config(config: ConfigurationPackage) -> Config { let settings = config.get_settings(); settings.sections.iter().for_each(|section| { info!("Section: {}", section.name); @@ -103,22 +136,16 @@ fn validate_configs(actctx: &CodePackageActivationContext) { assert_eq!(v.to_string_lossy(), "Value1"); assert!(!encrypt); + // TODO: add a overrideable parameter in the manifest / settings and log it here + // Use the config framework let source = FabricConfigSource::new(config); - let s = Config::builder() + + Config::builder() .add_source(source) .build() .inspect_err(|e| info!("config build failed: {}", e)) - .unwrap(); - let val = s.get::("my_config_section.my_string").unwrap(); - info!("entry: {}", val); - // note that the config name lookup is case sensitive for struct fields. - let settings = s.try_deserialize::().unwrap(); - info!("settings: {:?}", settings); - let sect = settings.my_config_section; - assert_eq!(sect.my_string, "Value1"); - assert!(sect.my_bool); - assert_eq!(sect.my_int, 99); + .unwrap() } /// Send health ok to SF to validate health reporting code diff --git a/crates/samples/no_default_features/src/lib.rs b/crates/samples/no_default_features/src/lib.rs index 05fb2f83..5763ed06 100644 --- a/crates/samples/no_default_features/src/lib.rs +++ b/crates/samples/no_default_features/src/lib.rs @@ -10,11 +10,25 @@ //! //! This sample demonstrates it is possible to use the library with default-features = false and ensures that that scenario remains compiling as PRs go into the repository. //! -use mssf_core::runtime::CodePackageActivationContext; + +use mssf_core::runtime::{package_change::PackageChangeEvent, CodePackageActivationContext}; #[no_mangle] fn test_fn() { - // Make sure we link something - // - let my_ctx = CodePackageActivationContext::create(); - my_ctx.unwrap(); + let my_ctx = CodePackageActivationContext::create().unwrap(); + + // One might wish to use such a callback to e.g. trigger custom handling of configuration changes + // This doesn't require the config feature to be enabled + let _handler = my_ctx.register_configuration_package_change_handler( |c| + { + let (some_package, change_type) = match c + { + PackageChangeEvent::Addition { new_package } => (new_package, "Addition"), + PackageChangeEvent::Removal { previous_package } => (previous_package, "Removal"), + PackageChangeEvent::Modification { previous_package: _, new_package } => (new_package, "Modification"), + }; + let changed_package_name = some_package.get_description().name.to_string_lossy(); + let changed_package_str = &changed_package_name; + println!("Received config package change of type {change_type:?} to package {changed_package_str}"); + } + ).unwrap(); }