refactor(app): improve component factory and LSP server lifecycle

Author: tembleking

src/app/component_factory.rs

@@ -3,6 +3,7 @@ use std::env::VarError;
 use bollard::Docker;
 use serde::Deserialize;
 use thiserror::Error;
+use tower_lsp::jsonrpc::{Error as LspError, ErrorCode};
 
 use crate::infra::{DockerImageBuilder, SysdigAPIToken, SysdigImageScanner};
 
@@ -17,19 +18,14 @@ pub struct SysdigConfig {
     api_token: Option<SysdigAPIToken>,
 }
 
-#[derive(Clone, Default)]
+#[derive(Clone)]
 pub struct ComponentFactory {
-    config: Option<Config>,
-
-    scanner: Option<SysdigImageScanner>,
-    builder: Option<DockerImageBuilder>,
+    scanner: SysdigImageScanner,
+    builder: DockerImageBuilder,
 }
 
 #[derive(Error, Debug)]
 pub enum ComponentFactoryError {
-    #[error("the configuration has not been provided")]
-    ConfigurationNotProvided,
-
     #[error("unable to retrieve sysdig api token from env var: {0}")]
     UnableToRetrieveAPITokenFromEnvVar(#[from] VarError),
 
@@ -38,43 +34,48 @@ pub enum ComponentFactoryError {
 }
 
 impl ComponentFactory {
-    pub fn initialize_with(&mut self, config: Config) {
-        self.config.replace(config);
-        self.scanner.take();
-    }
-
-    pub fn image_scanner(&mut self) -> Result<SysdigImageScanner, ComponentFactoryError> {
-        if self.scanner.is_some() {
-            return Ok(self.scanner.clone().unwrap());
-        }
-
-        let Some(config) = &self.config else {
-            return Err(ComponentFactoryError::ConfigurationNotProvided);
-        };
-
+    pub fn new(config: Config) -> Result<Self, ComponentFactoryError> {
         let token = config
             .sysdig
             .api_token
             .clone()
             .map(Ok)
             .unwrap_or_else(|| std::env::var("SECURE_API_TOKEN").map(SysdigAPIToken))?;
 
-        let image_scanner = SysdigImageScanner::new(config.sysdig.api_url.clone(), token);
+        let scanner = SysdigImageScanner::new(config.sysdig.api_url.clone(), token);
 
-        self.scanner.replace(image_scanner);
-        Ok(self.scanner.clone().unwrap())
+        let docker_client = Docker::connect_with_local_defaults()?;
+        let builder = DockerImageBuilder::new(docker_client);
+
+        Ok(Self { scanner, builder })
     }
 
-    pub fn image_builder(&mut self) -> Result<DockerImageBuilder, ComponentFactoryError> {
-        if self.builder.is_some() {
-            return Ok(self.builder.clone().unwrap());
-        }
+    pub fn image_scanner(&self) -> &SysdigImageScanner {
+        &self.scanner
+    }
 
-        let docker_client = Docker::connect_with_local_defaults()?;
-        let image_builder = DockerImageBuilder::new(docker_client);
+    pub fn image_builder(&self) -> &DockerImageBuilder {
+        &self.builder
+    }
+}
 
-        self.builder.replace(image_builder);
-        Ok(self.builder.clone().unwrap())
+impl From<ComponentFactoryError> for LspError {
+    fn from(err: ComponentFactoryError) -> Self {
+        let (code, message) = match err {
+            ComponentFactoryError::UnableToRetrieveAPITokenFromEnvVar(e) => (
+                ErrorCode::InternalError,
+                format!("Could not read SECURE_API_TOKEN from environment: {}", e),
+            ),
+            ComponentFactoryError::DockerClientError(e) => (
+                ErrorCode::InternalError,
+                format!("Failed to connect to Docker: {}", e),
+            ),
+        };
+        LspError {
+            code,
+            message: message.into(),
+            data: None,
+        }
     }
 }
 
@@ -83,32 +84,8 @@ mod test {
     use super::{ComponentFactory, Config};
 
     #[test]
-    fn it_loads_the_factory_uninit() {
-        let factory = ComponentFactory::default();
-
-        assert!(factory.config.is_none());
-    }
-
-    #[test]
-    fn it_fails_to_create_the_scanner_without_config() {
-        let mut factory = ComponentFactory::default();
-
-        assert!(factory.image_scanner().is_err());
-    }
-
-    #[test]
-    fn it_creates_a_scanner_after_initializing() {
-        let mut factory = ComponentFactory::default();
-
-        factory.initialize_with(Config::default());
-
-        assert!(factory.image_scanner().is_ok());
-    }
-
-    #[test]
-    fn it_creates_a_builder_without_config() {
-        let mut factory = ComponentFactory::default();
-
-        assert!(factory.image_builder().is_ok());
+    fn it_creates_a_factory() {
+        let factory = ComponentFactory::new(Config::default());
+        assert!(factory.is_ok());
     }
 }

src/app/lsp_server/lsp_server_inner.rs

@@ -42,17 +42,16 @@ impl<C> LSPServerInner<C>
 where
     C: LSPClient + Send + Sync + 'static,
 {
-    async fn initialize_component_factory_with(&mut self, config: &Value) -> Result<()> {
-        let Ok(config) = serde_json::from_value::<Config>(config.clone()) else {
-            return Err(Error::internal_error()
-                .with_message(format!("unable to transform json into config: {config}")));
-        };
+    fn update_component_factory(&mut self, config: &Value) -> Result<()> {
+        let config = serde_json::from_value::<Config>(config.clone()).map_err(|e| {
+            Error::internal_error()
+                .with_message(format!("unable to transform json into config: {e}"))
+        })?;
 
         debug!("updating with configuration: {config:?}");
 
-        let mut factory = ComponentFactory::default();
-        factory.initialize_with(config);
-        self.component_factory = Some(factory);
+        let factory = ComponentFactory::new(config)?;
+        self.component_factory.replace(factory);
 
         debug!("updated configuration");
         Ok(())
@@ -89,7 +88,7 @@ where
             });
         };
 
-        self.initialize_component_factory_with(&config).await?;
+        self.update_component_factory(&config)?;
 
         Ok(InitializeResult {
             capabilities: ServerCapabilities {
@@ -118,9 +117,7 @@ where
     }
 
     pub async fn did_change_configuration(&mut self, params: DidChangeConfigurationParams) {
-        let _ = self
-            .initialize_component_factory_with(&params.settings)
-            .await;
+        let _ = self.update_component_factory(&params.settings);
     }
 
     pub async fn did_open(&self, params: DidOpenTextDocumentParams) {
@@ -165,37 +162,29 @@ where
         Ok(Some(code_lenses))
     }
 
-    fn component_factory_mut(&mut self) -> Result<&mut ComponentFactory> {
-        self.component_factory
-            .as_mut()
-            .ok_or_else(|| Error::internal_error().with_message("LSP not initialized"))
-    }
-
     async fn execute_base_image_scan(
-        &mut self,
+        &self,
         location: tower_lsp::lsp_types::Location,
         image: String,
     ) -> Result<()> {
-        let image_scanner = self.component_factory_mut()?.image_scanner().map_err(|e| {
-            Error::internal_error().with_message(format!("unable to create image scanner: {e}"))
-        })?;
-        ScanBaseImageCommand::new(&image_scanner, &self.interactor, location, image)
+        let factory = self
+            .component_factory
+            .as_ref()
+            .ok_or_else(|| Error::internal_error().with_message("LSP not initialized"))?;
+        let image_scanner = factory.image_scanner();
+        ScanBaseImageCommand::new(image_scanner, &self.interactor, location, image)
             .execute()
             .await
     }
 
-    async fn execute_build_and_scan(
-        &mut self,
-        location: tower_lsp::lsp_types::Location,
-    ) -> Result<()> {
-        let factory = self.component_factory_mut()?;
-        let image_scanner = factory.image_scanner().map_err(|e| {
-            Error::internal_error().with_message(format!("unable to create image scanner: {e}"))
-        })?;
-        let image_builder = factory.image_builder().map_err(|e| {
-            Error::internal_error().with_message(format!("unable to create image builder: {e}"))
-        })?;
-        BuildAndScanCommand::new(&image_builder, &image_scanner, &self.interactor, location)
+    async fn execute_build_and_scan(&self, location: tower_lsp::lsp_types::Location) -> Result<()> {
+        let factory = self
+            .component_factory
+            .as_ref()
+            .ok_or_else(|| Error::internal_error().with_message("LSP not initialized"))?;
+        let image_scanner = factory.image_scanner();
+        let image_builder = factory.image_builder();
+        BuildAndScanCommand::new(image_builder, image_scanner, &self.interactor, location)
             .execute()
             .await
     }
@@ -211,7 +200,7 @@ where
         }
     }
 
-    pub async fn execute_command(&mut self, params: ExecuteCommandParams) -> Result<Option<Value>> {
+    pub async fn execute_command(&self, params: ExecuteCommandParams) -> Result<Option<Value>> {
         let command: SupportedCommands = params.try_into()?;
         let command_name = command.to_string();
 

src/app/lsp_server/mod.rs

@@ -85,7 +85,7 @@ where
     }
 
     async fn execute_command(&self, params: ExecuteCommandParams) -> Result<Option<Value>> {
-        self.inner.write().await.execute_command(params).await
+        self.inner.read().await.execute_command(params).await
     }
 
     async fn shutdown(&self) -> Result<()> {