Re: [PATCH datacenter-manager v2 08/20] context: introduce a ContextFactory to build application context

[Dominik Csapak <d.csapak@proxmox.com>]

comment inline

On 8/20/26 4:52 PM, Lukas Wagner wrote:
> This makes it easier to selectively override behavior for the fake
> remote feature, as well as integration tests.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>   server/src/context/default.rs       |  5 ++
>   server/src/context/faked_remotes.rs | 42 ++++++++++++++++
>   server/src/context/mod.rs           | 78 ++++++++++++++++-------------
>   3 files changed, 90 insertions(+), 35 deletions(-)
>   create mode 100644 server/src/context/default.rs
>   create mode 100644 server/src/context/faked_remotes.rs
> 
> diff --git a/server/src/context/default.rs b/server/src/context/default.rs
> new file mode 100644
> index 00000000..e9c29e53
> --- /dev/null
> +++ b/server/src/context/default.rs
> @@ -0,0 +1,5 @@
> +use crate::context::ContextFactory;
> +
> +pub struct DefaultContextFactory;
> +
> +impl ContextFactory for DefaultContextFactory {}
> diff --git a/server/src/context/faked_remotes.rs b/server/src/context/faked_remotes.rs
> new file mode 100644
> index 00000000..b2ae1c3f
> --- /dev/null
> +++ b/server/src/context/faked_remotes.rs
> @@ -0,0 +1,42 @@
> +use std::sync::Arc;
> +
> +use anyhow::{Context, Error};
> +use pdm_config::remotes::RemoteConfig;
> +
> +use crate::connection::ClientFactory;
> +use crate::context::ContextFactory;
> +use crate::test_support::fake_remote::{FakeClientFactory, FakeRemoteConfig};
> +
> +pub struct FakedRemoteContextFactory(FakeRemoteConfig);
> +
> +impl FakedRemoteContextFactory {
> +    pub fn new() -> Result<Self, Error> {
> +        let path = std::env::var("PDM_FAKED_REMOTE_CONFIG").context(
> +            "compiled with remote_config = 'faked', but PDM_FAKED_REMOTE_CONFIG not set",
> +        )?;
> +
> +        log::info!("using fake remotes from {path:?}");
> +        let config = FakeRemoteConfig::from_json_config(&path)
> +            .context("could not deserialize fake remote config")?;
> +
> +        Ok(Self(config))
> +    }
> +}
> +
> +impl ContextFactory for FakedRemoteContextFactory {
> +    fn make_client_factory(&self) -> Result<Arc<dyn ClientFactory + Send + Sync>, Error> {
> +        Ok(Arc::new(FakeClientFactory {
> +            config: self.0.clone(),
> +        }))
> +    }

here there is a client factory defined for the faked remote context, but...

> +
> +    fn make_remote_config(&self) -> Result<Box<dyn RemoteConfig + Send + Sync>, Error> {
> +        Ok(Box::new(self.0.clone()))
> +    }
> +
> +    // No need to override subscription_key_config_impl here.
> +    //
> +    // The subscription key pool is product-only (PDM stores its own pool of
> +    // keys regardless of how remotes are mocked or not), so initialise it on
> +    // both paths.
> +}
> diff --git a/server/src/context/mod.rs b/server/src/context/mod.rs
> index a4afcddd..24d653c3 100644
> --- a/server/src/context/mod.rs
> +++ b/server/src/context/mod.rs
> @@ -2,48 +2,56 @@
>   //!
>   //! Make sure to call `init` *once* when starting up the API server.
>   
> +use std::sync::Arc;
> +
>   use anyhow::Error;
> +use pdm_config::{remotes::RemoteConfig, subscriptions::SubscriptionKeyConfig};
>   
> -use crate::connection;
> +use crate::connection::{self, ClientFactory};
>   
> -/// Dependency-inject production remote-config implementation and remote client factory
> -#[allow(dead_code)]
> -fn default_remote_setup() {
> -    pdm_config::remotes::init(Box::new(pdm_config::remotes::DefaultRemoteConfig));
> -    connection::init(Box::new(connection::DefaultClientFactory));
> -}
> +#[cfg(remote_config = "faked")]
> +mod faked_remotes;
> +
> +#[cfg(not(remote_config = "faked"))]
> +mod default;
>   
>   /// Dependency-inject concrete implementations needed at runtime.
>   pub fn init() -> Result<(), Error> {
> -    // The subscription key pool is product-only (PDM stores its own pool of
> -    // keys regardless of how remotes are mocked or not), so initialise it on
> -    // both paths.
> -    pdm_config::subscriptions::init(Box::new(
> -        pdm_config::subscriptions::DefaultSubscriptionKeyConfig,
> -    ));
> +    let factory = context_factory()?;
>   
> -    #[cfg(remote_config = "faked")]
> -    {
> -        use anyhow::bail;
> -
> -        use crate::test_support::fake_remote;
> -
> -        match std::env::var("PDM_FAKED_REMOTE_CONFIG") {
> -            Ok(path) => {
> -                log::info!("using fake remotes from {path:?}");
> -                let config = fake_remote::FakeRemoteConfig::from_json_config(&path)?;
> -                pdm_config::remotes::init(Box::new(config.clone()));
> -                connection::init(Box::new(fake_remote::FakeClientFactory { config }));
> -            }
> -            Err(_) => {
> -                bail!("compiled with remote_config = 'faked', but PDM_FAKED_REMOTE_CONFIG not set")
> -            }
> -        }
> -    }
> -    #[cfg(not(remote_config = "faked"))]
> -    {
> -        default_remote_setup();
> -    }
> +    pdm_config::subscriptions::init(factory.make_subscription_key_config()?);
> +    pdm_config::remotes::init(factory.make_remote_config()?);
> +    // FIXME: Rather let connection use an Application context object from here
> +    connection::init(Box::new(connection::DefaultClientFactory));

here we always use the defaultclientfactory?

shouldn't this here be:

factory.make_client_factory()
?

i know this is changed in the next commit anyway, but
IMHO it make sense to keep the functionality for intermediate
commits (so a bisect works...)


>   
>       Ok(())
>   }
> +
> +pub trait ContextFactory {
> +    fn make_client_factory(&self) -> Result<Arc<dyn ClientFactory + Send + Sync>, Error> {
> +        Ok(Arc::new(connection::DefaultClientFactory))
> +    }
> +
> +    fn make_remote_config(&self) -> Result<Box<dyn RemoteConfig + Send + Sync>, Error> {
> +        Ok(Box::new(pdm_config::remotes::DefaultRemoteConfig))
> +    }
> +
> +    fn make_subscription_key_config(
> +        &self,
> +    ) -> Result<Box<dyn SubscriptionKeyConfig + Send + Sync>, Error> {
> +        Ok(Box::new(
> +            pdm_config::subscriptions::DefaultSubscriptionKeyConfig,
> +        ))
> +    }
> +}
> +
> +fn context_factory() -> Result<impl ContextFactory, Error> {
> +    #[cfg(remote_config = "faked")]
> +    {
> +        faked_remotes::FakedRemoteContextFactory::new()
> +    }
> +    #[cfg(not(remote_config = "faked"))]
> +    {
> +        Ok(default::DefaultContextFactory)
> +    }
> +}