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

[FabricRuntime] Support Package change notification callbacks #96

Merged
merged 18 commits into from
Dec 28, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
2 changes: 2 additions & 0 deletions crates/libs/core/src/client/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>
where
T: ServiceNotificationEventHandler,
Expand Down Expand Up @@ -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<T>
where
T: Fn(&ServiceNotification) -> crate::Result<()> + 'static,
Expand Down
26 changes: 25 additions & 1 deletion crates/libs/core/src/runtime/activation_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,16 @@ use crate::{
Error, WString, PCWSTR,
};

use super::config::ConfigurationPackage;
use super::{
config::ConfigurationPackage,
package_change::{
config::{
ConfigurationPackageChangeCallbackAutoHandle,
ConfigurationPackageChangeEventHandlerBridge, LambdaConfigurationPackageEventHandler,
},
ConfigurationPackageChangeEvent,
},
};

#[derive(Debug, Clone)]
pub struct CodePackageActivationContext {
Expand Down Expand Up @@ -132,6 +141,21 @@ impl CodePackageActivationContext {
pub fn get_com(&self) -> IFabricCodePackageActivationContext6 {
self.com_impl.clone()
}

pub fn register_config_package_change_handler<T>(
&self,
handler: T,
) -> crate::Result<ConfigurationPackageChangeCallbackAutoHandle>
where
T: Fn(&ConfigurationPackageChangeEvent) -> crate::Result<()> + 'static,
{
let lambda_handler = LambdaConfigurationPackageEventHandler::new(handler);
let bridge = ConfigurationPackageChangeEventHandlerBridge::new(lambda_handler);
ConfigurationPackageChangeCallbackAutoHandle::register_config_package_change_handler(
self.get_com(),
bridge.into(),
)
}
}

impl From<IFabricCodePackageActivationContext6> for CodePackageActivationContext {
Expand Down
3 changes: 3 additions & 0 deletions crates/libs/core/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
168 changes: 168 additions & 0 deletions crates/libs/core/src/runtime/package_change/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
// ------------------------------------------------------------
// 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::{
IFabricCodePackageActivationContext6, 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) -> crate::Result<()>;
}

// 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<T>
where
T: ConfigurationPackageChangeEventHandler,
{
inner: T,
}

impl<T> ConfigurationPackageChangeEventHandlerBridge<T>
where
T: ConfigurationPackageChangeEventHandler,
{
pub fn new(inner: T) -> Self {
Self { inner }
}
}

impl<T> IFabricConfigurationPackageChangeHandler_Impl
for ConfigurationPackageChangeEventHandlerBridge_Impl<T>
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 };
// TODO: unwrap, or should we change the return type of the lambda to be the empty type?
cgettys-microsoft marked this conversation as resolved.
Show resolved Hide resolved
self.inner.on_change(&event).unwrap();
}

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).unwrap();
}

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).unwrap();
}
}

/// 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<T>
where
T: Fn(&ConfigurationPackageChangeEvent) -> crate::Result<()> + 'static,
{
f: T,
}

impl<T> LambdaConfigurationPackageEventHandler<T>
where
T: Fn(&ConfigurationPackageChangeEvent) -> crate::Result<()> + 'static,
{
pub fn new(f: T) -> Self {
Self { f }
}
}

impl<T> ConfigurationPackageChangeEventHandler for LambdaConfigurationPackageEventHandler<T>
where
T: Fn(&ConfigurationPackageChangeEvent) -> crate::Result<()> + 'static,
{
fn on_change(&self, change: &ConfigurationPackageChangeEvent) -> crate::Result<()> {
(self.f)(change)
}
}

pub struct ConfigurationPackageChangeCallbackHandle(i64);

impl ConfigurationPackageChangeCallbackHandle {
pub const unsafe fn from_com(com: i64) -> Self {
Self(com)
}

pub fn register_configuration_package_change_handler(
activation_context: &IFabricCodePackageActivationContext6,
implementation: IFabricConfigurationPackageChangeHandler,
) -> crate::Result<Self> {
let raw_handle = unsafe {
activation_context.RegisterConfigurationPackageChangeHandler(&implementation)
}?;
Ok(unsafe { Self::from_com(raw_handle) })
}

pub unsafe fn unregister_configuration_package_change_handler(
self,
activation_context: &IFabricCodePackageActivationContext6,
) {
// SAFETY: caller taking responsibility for ensuring this is the correct activation context and a live handle.
unsafe { activation_context.UnregisterConfigurationPackageChangeHandler(self.0) }.unwrap();
}
}

/// This struct ensures that the handle is retained and deregistered before the implementation is dropped
pub struct ConfigurationPackageChangeCallbackAutoHandle {
activation_context: IFabricCodePackageActivationContext6,
handle: ConfigurationPackageChangeCallbackHandle,
}

impl ConfigurationPackageChangeCallbackAutoHandle {
pub fn register_config_package_change_handler(
activation_context: IFabricCodePackageActivationContext6,
implementation: IFabricConfigurationPackageChangeHandler,
) -> crate::Result<Self> {
let handle = ConfigurationPackageChangeCallbackHandle::register_configuration_package_change_handler(&activation_context, implementation)?;
Ok(Self {
activation_context,
handle,
})
}
}

impl Drop for ConfigurationPackageChangeCallbackAutoHandle {
fn drop(&mut self) {
// Note: we don't use the helper as the "raw" handle type should not be cloneable, and we can't mvoe out of a mut reference
// So we just do what it would do anyway
unsafe {
self.activation_context
.UnregisterConfigurationPackageChangeHandler(self.handle.0)
}
.unwrap();
}
}
17 changes: 17 additions & 0 deletions crates/libs/core/src/runtime/package_change/mod.rs
Original file line number Diff line number Diff line change
@@ -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<T> {
Addition { new_package: T },
Removal { previous_package: T },
Modification { previous_package: T, new_package: T },
}

pub type ConfigurationPackageChangeEvent = PackageChangeEvent<super::config::ConfigurationPackage>;
25 changes: 20 additions & 5 deletions crates/samples/no_default_features/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,26 @@
//!
//! 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_config_package_change_handler( |c|
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

U should move this to one of the example apps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? this is one of the samples

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you mean one of the full applications.
Yeah, that could make sense.
I do want to ensure this continues to build without those features though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logical example to add it to would be echomain

As we could demonstrate how to hook it up to trigger a Config reload

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given I'd like to have this build here as well, can I defer that to a second PR?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

U can keep it here or add to the working app.
But note that this code here cannot run because it is not launched by SF, and we are not packaging it in an app.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it to echomain as well (but haven't had a chance to test that app yet)
Right, point of this is to make sure we can package w/o tokio

{
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}");
Ok(())
}
).unwrap();
}