Gracefully handle invalid state

implementations/rust/ockam/ockam_api/src/cli_state/credentials.rs

@@ -78,6 +78,12 @@ mod traits {
         }
     }
 
+    impl StateItemFileTrait for CredentialState {
+        fn path(&self) -> &PathBuf {
+            &self.path
+        }
+    }
+
     #[async_trait]
     impl StateItemTrait for CredentialState {
         type Config = CredentialConfig;
@@ -89,15 +95,13 @@ mod traits {
             Ok(Self { name, path, config })
         }
 
-        fn load(path: PathBuf) -> Result<Self> {
+        fn load<'a>(path: PathBuf) -> Result<Self> {
             let name = file_stem(&path)?;
             let contents = std::fs::read_to_string(&path)?;
-            let config = serde_json::from_str(&contents)?;
-            Ok(Self { name, path, config })
-        }
-
-        fn path(&self) -> &PathBuf {
-            &self.path
+            match serde_json::from_str(&contents) {
+                Ok(config) => Ok(Self { name, path, config }),
+                Err(error) => Err(error.handle_file_parse_error(path))
+            }
         }
 
         fn config(&self) -> &Self::Config {

implementations/rust/ockam/ockam_api/src/cli_state/identities.rs

@@ -271,6 +271,12 @@ mod traits {
         }
     }
 
+    impl StateItemFileTrait for IdentityState {
+        fn path(&self) -> &PathBuf {
+            &self.path
+        }
+    }
+
     #[async_trait]
     impl StateItemTrait for IdentityState {
         type Config = IdentityConfig;
@@ -291,18 +297,16 @@ mod traits {
         fn load(path: PathBuf) -> Result<Self> {
             let name = file_stem(&path)?;
             let contents = std::fs::read_to_string(&path)?;
-            let config = serde_json::from_str(&contents)?;
             let data_path = IdentityState::build_data_path(&path);
-            Ok(Self {
-                name,
-                path,
-                data_path,
-                config,
-            })
-        }
-
-        fn path(&self) -> &PathBuf {
-            &self.path
+            match serde_json::from_str(&contents) {
+                Ok(config) => Ok(Self {
+                    name,
+                    path,
+                    data_path,
+                    config,
+                }),
+                Err(error) => Err(error.handle_file_parse_error(path)),
+            }
         }
 
         fn config(&self) -> &Self::Config {

implementations/rust/ockam/ockam_api/src/cli_state/mod.rs

@@ -41,6 +41,13 @@ pub enum CliStateError {
     #[diagnostic(code("OCK500"))]
     Serde(#[from] serde_json::Error),
 
+    #[error("A file is invalid")]
+    #[diagnostic(code("OCK500"))]
+    InvalidFile{
+        source: serde_json::Error,
+        file: Box<dyn StateItemFileTrait>
+    },
+
     #[error(transparent)]
     #[diagnostic(code("OCK500"))]
     Ockam(#[from] ockam_core::Error),
@@ -99,6 +106,18 @@ impl From<CliStateError> for ockam_core::Error {
     }
 }
 
+// Default Invalid File struct
+#[derive(Debug, Clone, Eq, PartialEq)]
+struct InvalidItem {
+    path: PathBuf
+}
+
+impl StateItemFileTrait for InvalidItem {
+    fn path(&self) -> &PathBuf {
+        &self.path
+    }
+}
+
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct CliState {
     pub vaults: VaultsState,
@@ -242,7 +261,12 @@ impl CliState {
     pub fn delete_identity(&self, identity_state: IdentityState) -> Result<()> {
         // Abort if identity is being used by some running node.
         for node in self.nodes.list()? {
-            if node.config().identity_config()?.identifier() == identity_state.identifier() {
+            if node
+                .config()
+                .identity_config()?
+                .identifier()
+                == identity_state.identifier()
+            {
                 return Err(CliStateError::InvalidOperation(format!(
                     "Can't delete identity '{}' as it's being used by node '{}'",
                     &identity_state.name(),
@@ -342,6 +366,7 @@ impl CliState {
     /// This project should be the project that is created at the end of the enrollment procedure
     pub fn is_enrolled(&self) -> Result<bool> {
         let identity_state = self.identities.default()?;
+
         if !identity_state.is_enrolled() {
             return Ok(false);
         }
@@ -570,6 +595,7 @@ mod tests {
             let name = random_name();
             let vault_state = sut.vaults.get(&vault_name).unwrap();
             let vault: Vault = vault_state.get().await.unwrap();
+
             let identities = Identities::builder()
                 .with_vault(vault)
                 .with_identities_repository(sut.identities.identities_repository().await?)

implementations/rust/ockam/ockam_api/src/cli_state/nodes.rs

@@ -1,7 +1,7 @@
 use super::Result;
 use crate::cli_state::{
     CliState, CliStateError, IdentityConfig, IdentityState, ProjectConfig, ProjectConfigCompact,
-    StateDirTrait, StateItemTrait, VaultState,
+    StateDirTrait, StateItemFileTrait, StateItemTrait, VaultState,
 };
 use crate::config::lookup::ProjectLookup;
 use crate::nodes::models::transport::CreateTransportJson;
@@ -57,6 +57,19 @@ impl NodesState {
     }
 }
 
+#[derive(Debug, Clone, Eq, PartialEq)]
+struct InvalidNodeState {
+    path: PathBuf,
+}
+
+impl InvalidNodeState {
+    fn _delete(&self) -> Result<()> {
+        std::fs::remove_dir_all(&self.path)?;
+        std::fs::remove_dir(&self.path)?;
+        Ok(())
+    }
+}
+
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct NodeState {
     name: String,
@@ -508,6 +521,26 @@ mod traits {
         }
     }
 
+    impl StateItemFileTrait for NodeState {
+        fn delete(&self) -> Result<()> {
+            self._delete(false)
+        }
+
+        fn path(&self) -> &PathBuf {
+            &self.path
+        }
+    }
+
+    impl StateItemFileTrait for InvalidNodeState {
+        fn delete(&self) -> Result<()> {
+            self._delete()
+        }
+
+        fn path(&self) -> &PathBuf {
+            &self.path
+        }
+    }
+
     #[async_trait]
     impl StateItemTrait for NodeState {
         type Config = NodeConfig;
@@ -535,10 +568,18 @@ mod traits {
         fn load(path: PathBuf) -> Result<Self> {
             let paths = NodePaths::new(&path);
             let name = file_stem(&path)?;
-            let setup = {
+            let (path, setup) = {
                 let contents = std::fs::read_to_string(paths.setup())?;
-                serde_json::from_str(&contents)?
-            };
+
+                // I may be going overkill with trying to avoid temporarys, please forgive this humble new rustatcan
+                match serde_json::from_str(&contents) {
+                    Ok(config) => Ok((path, config)),
+                    Err(source) => Err(CliStateError::InvalidFile {
+                        source,
+                        file: Box::new(InvalidNodeState { path })
+                    })
+                }
+            }?;
             let version = {
                 let contents = std::fs::read_to_string(paths.version())?;
                 contents.parse::<ConfigVersion>()?
@@ -557,14 +598,6 @@ mod traits {
             })
         }
 
-        fn delete(&self) -> Result<()> {
-            self._delete(false)
-        }
-
-        fn path(&self) -> &PathBuf {
-            &self.path
-        }
-
         fn config(&self) -> &Self::Config {
             &self.config
         }

implementations/rust/ockam/ockam_api/src/cli_state/projects.rs

@@ -142,6 +142,12 @@ mod traits {
         }
     }
 
+    impl StateItemFileTrait for ProjectState {
+        fn path(&self) -> &PathBuf {
+            &self.path
+        }
+    }
+
     #[async_trait]
     impl StateItemTrait for ProjectState {
         type Config = ProjectConfig;
@@ -156,12 +162,10 @@ mod traits {
         fn load(path: PathBuf) -> Result<Self> {
             let name = file_stem(&path)?;
             let contents = std::fs::read_to_string(&path)?;
-            let config = serde_json::from_str(&contents)?;
-            Ok(Self { name, path, config })
-        }
-
-        fn path(&self) -> &PathBuf {
-            &self.path
+            match serde_json::from_str(&contents) {
+                Ok(config) => Ok(Self { name, path, config }),
+                Err(source) => Err(source.handle_file_parse_error(path))
+            }
         }
 
         fn config(&self) -> &Self::Config {

implementations/rust/ockam/ockam_api/src/cli_state/spaces.rs

@@ -71,6 +71,12 @@ mod traits {
         }
     }
 
+    impl StateItemFileTrait for SpaceState {
+        fn path(&self) -> &PathBuf {
+            &self.path
+        }
+    }
+
     #[async_trait]
     impl StateItemTrait for SpaceState {
         type Config = SpaceConfig;
@@ -85,12 +91,10 @@ mod traits {
         fn load(path: PathBuf) -> Result<Self> {
             let name = file_stem(&path)?;
             let contents = std::fs::read_to_string(&path)?;
-            let config = serde_json::from_str(&contents)?;
-            Ok(Self { name, path, config })
-        }
-
-        fn path(&self) -> &PathBuf {
-            &self.path
+            match serde_json::from_str(&contents) {
+                Ok(config) => Ok(Self { name, path, config }),
+                Err(source) => Err(source.handle_file_parse_error(path))
+            }
         }
 
         fn config(&self) -> &Self::Config {

implementations/rust/ockam/ockam_api/src/cli_state/traits.rs

@@ -1,7 +1,8 @@
-use crate::cli_state::{file_stem, CliState, CliStateError};
+use crate::cli_state::{file_stem, CliState, CliStateError, InvalidItem};
 use ockam_core::errcode::{Kind, Origin};
 use ockam_core::{async_trait, Error};
 use serde::{Deserialize, Serialize};
+use std::fmt::Debug;
 use std::path::{Path, PathBuf};
 
 use super::Result;
@@ -238,10 +239,22 @@ pub trait StateDirTrait: Sized + Send + Sync {
     }
 }
 
+/// This trait defines the methods to retrieve an file from a state directory,
+/// but does not validate if it's a correct config.
+/// Provides a way to delete invalid files gracefully.
+#[async_trait]
+pub trait StateItemFileTrait: Send + Sync + Debug {
+    fn delete(&self) -> Result<()> {
+        std::fs::remove_file(self.path())?;
+        Ok(())
+    }
+    fn path(&self) -> &PathBuf;
+}
+
 /// This trait defines the methods to retrieve an item from a state directory.
 /// The details of the item are defined in the `Config` type.
 #[async_trait]
-pub trait StateItemTrait: Sized + Send {
+pub trait StateItemTrait: Sized + Send + StateItemFileTrait {
     type Config: Serialize + for<'a> Deserialize<'a> + Send;
 
     /// Create a new item with the given config.
@@ -256,19 +269,29 @@ pub trait StateItemTrait: Sized + Send {
         std::fs::write(self.path(), contents)?;
         Ok(())
     }
-    fn delete(&self) -> Result<()> {
-        std::fs::remove_file(self.path())?;
-        Ok(())
-    }
-    fn path(&self) -> &PathBuf;
     fn config(&self) -> &Self::Config;
 }
 
+pub trait HandleFileParseError {
+    fn handle_file_parse_error(self, path: PathBuf) -> CliStateError;
+}
+
+impl HandleFileParseError for serde_json::Error {
+    fn handle_file_parse_error(self, path: PathBuf) -> CliStateError {
+        CliStateError::InvalidFile {
+            source: self,
+            file: Box::new(InvalidItem { path }),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::cli_state::{StateDirTrait, StateItemTrait};
     use std::path::{Path, PathBuf};
 
+    use super::StateItemFileTrait;
+
     #[test]
     fn test_is_item_path() {
         let config = TestConfig::new(Path::new("dir"));
@@ -297,10 +320,18 @@ mod tests {
         }
     }
 
+    #[derive(Debug)]
     struct TestConfigItem {
         path: PathBuf,
         config: String,
     }
+
+    impl StateItemFileTrait for TestConfigItem {
+        fn path(&self) -> &PathBuf {
+            &self.path
+        }
+    }
+
     impl StateItemTrait for TestConfigItem {
         type Config = String;
 
@@ -315,10 +346,6 @@ mod tests {
             })
         }
 
-        fn path(&self) -> &PathBuf {
-            &self.path
-        }
-
         fn config(&self) -> &Self::Config {
             &self.config
         }