[PATCH datacenter-manager v3 09/21] context: introduce a ContextFactory to build application context

[Lukas Wagner <l.wagner@proxmox.com>]

This makes it easier to selectively override behavior for the fake
remote feature, as well as integration tests.

The change from Box to Arc in the stored client factory is merely to
maintain bisectability, a future commit needs make_client_factory to
return an Arc.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v2:
    
    Use the context factory to build the client factory. This required an
    intermittent change from Box<...> to Arc<...> in the stored client
    factory, but this is changed anyways in the following commit.

 server/src/connection.rs                      |  4 +-
 server/src/context/default.rs                 |  5 ++
 server/src/context/faked_remotes.rs           | 42 ++++++++++
 server/src/context/mod.rs                     | 78 ++++++++++---------
 .../remote_collection_task.rs                 |  2 +-
 5 files changed, 93 insertions(+), 38 deletions(-)
 create mode 100644 server/src/context/default.rs
 create mode 100644 server/src/context/faked_remotes.rs

diff --git a/server/src/connection.rs b/server/src/connection.rs
index a63ea7da..140a1902 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -28,7 +28,7 @@ use pve_api_types::client::PveClientImpl;
 use crate::pbs_client::PbsClient;
 use crate::remote_cache::ConnectionState;
 
-static INSTANCE: OnceLock<Box<dyn ClientFactory + Send + Sync>> = OnceLock::new();
+static INSTANCE: OnceLock<Arc<dyn ClientFactory + Send + Sync>> = OnceLock::new();
 
 /// Connection Info returned from [`prepare_connect_client`]
 struct ConnectInfo {
@@ -470,7 +470,7 @@ pub async fn make_pbs_client_and_login(remote: &Remote) -> Result<Box<PbsClient<
 /// Initialize the [`ClientFactory`] instance.
 ///
 /// Will panic if the instance has already been set.
-pub fn init(instance: Box<dyn ClientFactory + Send + Sync>) {
+pub fn init(instance: Arc<dyn ClientFactory + Send + Sync>) {
     if INSTANCE.set(instance).is_err() {
         panic!("connection factory instance already set");
     }
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(),
+        }))
+    }
+
+    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..77c84f98 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(factory.make_client_factory()?);
 
     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)
+    }
+}
diff --git a/server/src/metric_collection/remote_collection_task.rs b/server/src/metric_collection/remote_collection_task.rs
index d243dcf1..9fe67371 100644
--- a/server/src/metric_collection/remote_collection_task.rs
+++ b/server/src/metric_collection/remote_collection_task.rs
@@ -558,7 +558,7 @@ pub(super) mod tests {
             // TODO: the client factory is currently stored in a OnceLock -
             // we can only set it from one test... Ideally we'd like to have the
             // option to set it in every single test if needed - task/thread local?
-            connection::init(Box::new(TestClientFactory { now }));
+            connection::init(Arc::new(TestClientFactory { now }));
         });
 
         now
-- 
2.47.3