refactor/error-types

src/dfx-core/src/config/model/dfinity.rs

@@ -29,11 +29,13 @@ use crate::error::socket_addr_conversion::SocketAddrConversionError;
 use crate::error::socket_addr_conversion::SocketAddrConversionError::{
     EmptyIterator, ParseSocketAddrFailed,
 };
-use crate::error::structured_file::StructuredFileError;
 use crate::error::structured_file::StructuredFileError::{
     DeserializeJsonFileFailed, ReadJsonFileFailed,
 };
-use crate::extension::manifest::custom_canister_type;
+use crate::error::structured_file::{
+    ReadConfigurationError, StructuredFileError, TransformConfigurationError,
+};
+use crate::extension::manifest::custom_canister_type::TransformConfiguration;
 use crate::json::save_json_file;
 use crate::json::structure::{PossiblyStr, SerdeVec};
 use byte_unit::Byte;
@@ -952,65 +954,70 @@ impl Config {
         Ok(None)
     }
 
-    fn from_file(
+    fn from_file<T: TransformConfiguration>(
         path: &Path,
-        dfx_version: &semver::Version,
-    ) -> Result<Config, StructuredFileError> {
+        transformer: &mut T,
+    ) -> Result<Config, ReadConfigurationError> {
         let content = crate::fs::read(path).map_err(ReadJsonFileFailed)?;
-        Config::from_slice(path.to_path_buf(), &content, dfx_version)
+        Config::from_slice(path.to_path_buf(), &content, transformer)
     }
 
-    pub fn from_dir(
+    pub fn from_dir<T: TransformConfiguration>(
         working_dir: &Path,
-        dfx_version: &semver::Version,
+        transformer: &mut T,
     ) -> Result<Option<Config>, LoadDfxConfigError> {
         let path = Config::resolve_config_path(working_dir)?;
-        path.map(|path| Config::from_file(&path, dfx_version))
+        path.map(|path| Config::from_file(&path, transformer))
             .transpose()
             .map_err(LoadFromFileFailed)
     }
 
-    pub fn from_current_dir(
-        dfx_version: &semver::Version,
+    pub fn from_current_dir<T: TransformConfiguration>(
+        transformer: &mut T,
     ) -> Result<Option<Config>, LoadDfxConfigError> {
         Config::from_dir(
             &std::env::current_dir().map_err(DetermineCurrentWorkingDirFailed)?,
-            dfx_version,
+            transformer,
         )
     }
 
-    fn from_slice(
+    fn from_slice<T: TransformConfiguration>(
         path: PathBuf,
         content: &[u8],
-        dfx_version: &semver::Version,
-    ) -> Result<Config, StructuredFileError> {
+        transformer: &mut T,
+    ) -> Result<Config, ReadConfigurationError> {
         let mut json: serde_json::Value = serde_json::from_slice(content)
             .map_err(|e| DeserializeJsonFileFailed(Box::new(path.clone()), e))?;
-        let extension_manager =
-            crate::extension::manager::ExtensionManager::new(dfx_version).unwrap();
-        custom_canister_type::transform_dfx_json_via_extension(&mut json, extension_manager)
-            .unwrap(); // TODO: error handling
+
+        transformer
+            .transform(&mut json)
+            .map_err(TransformConfigurationError::from)?;
+
         let config = serde_json::from_value(json.clone())
             .map_err(|e| DeserializeJsonFileFailed(Box::new(path.clone()), e))?;
         Ok(Config { path, json, config })
     }
 
     /// Create a configuration from a string.
     #[cfg(test)]
-    pub(crate) fn from_str(
-        content: &str,
-        dfx_version: &semver::Version,
-    ) -> Result<Config, StructuredFileError> {
-        Config::from_slice(PathBuf::from("-"), content.as_bytes(), dfx_version)
+    pub(crate) fn from_str(content: &str) -> Result<Config, ReadConfigurationError> {
+        let mut no_op_transformer =
+            crate::extension::manifest::custom_canister_type::NoopTransformConfiguration;
+        Config::from_slice(
+            PathBuf::from("-"),
+            content.as_bytes(),
+            &mut no_op_transformer,
+        )
     }
 
     #[cfg(test)]
     pub(crate) fn from_str_and_path(
         path: PathBuf,
         content: &str,
-        dfx_version: &semver::Version,
-    ) -> Result<Config, StructuredFileError> {
-        Config::from_slice(path, content.as_bytes(), dfx_version)
+    ) -> Result<Config, ReadConfigurationError> {
+        let mut no_op_transformer =
+            crate::extension::manifest::custom_canister_type::NoopTransformConfiguration;
+        Config::from_slice(path, content.as_bytes(), &mut no_op_transformer)
     }
 
     pub fn get_path(&self) -> &PathBuf {
@@ -1257,7 +1264,6 @@ mod tests {
                 }
             }
         }"#,
-            &semver::Version::new(0, 0, 0),
         )
         .unwrap();
 
@@ -1280,7 +1286,6 @@ mod tests {
                 }
             }
         }"#,
-            &semver::Version::new(0, 0, 0),
         )
         .unwrap();
 
@@ -1302,7 +1307,6 @@ mod tests {
                 }
             }
         }"#,
-            &semver::Version::new(0, 0, 0),
         )
         .unwrap();
 
@@ -1325,7 +1329,6 @@ mod tests {
                 }
             }
         }"#,
-            &semver::Version::new(0, 0, 0),
         )
         .unwrap();
 
@@ -1359,7 +1362,6 @@ mod tests {
                 }
               }
         }"#,
-            &semver::Version::new(0, 0, 0),
         )
         .unwrap();
 
@@ -1383,7 +1385,6 @@ mod tests {
                 }
               }
         }"#,
-            &semver::Version::new(0, 0, 0),
         )
         .unwrap();
         let config_interface = config_no_values.get_config();

src/dfx-core/src/error/load_dfx_config.rs

@@ -1,15 +1,15 @@
 use crate::error::fs::FsError;
-use crate::error::structured_file::StructuredFileError;
+use crate::error::structured_file::ReadConfigurationError;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum LoadDfxConfigError {
     #[error("Failed to resolve config path: {0}")]
     ResolveConfigPathFailed(FsError),
 
-    #[error("Failed to load dfx configuration: {0}")]
-    LoadFromFileFailed(StructuredFileError),
-
     #[error("Failed to determine current working dir: {0}")]
     DetermineCurrentWorkingDirFailed(std::io::Error),
+
+    #[error("Failed to load dfx configuration: {0}")]
+    LoadFromFileFailed(ReadConfigurationError),
 }

src/dfx-core/src/error/structured_file.rs

@@ -2,6 +2,8 @@ use crate::error::fs::FsError;
 use std::path::PathBuf;
 use thiserror::Error;
 
+use super::extension::ExtensionError;
+
 #[derive(Error, Debug)]
 pub enum StructuredFileError {
     #[error("Failed to parse contents of {0} as json: {1}")]
@@ -16,3 +18,19 @@ pub enum StructuredFileError {
     #[error("Failed to write JSON file: {0}")]
     WriteJsonFileFailed(FsError),
 }
+
+#[derive(Error, Debug)]
+pub enum ReadConfigurationError {
+    #[error(transparent)]
+    StructuredFile(#[from] StructuredFileError),
+    #[error(transparent)]
+    TransformConfiguration(#[from] TransformConfigurationError),
+}
+
+#[derive(Error, Debug)]
+pub enum TransformConfigurationError {
+    #[error("Configuration transformation failed: {0}")]
+    ConfigurationTransformationFailed(String), // Or another error type if necessary
+    #[error("Extension error: {0}")]
+    ExtensionError(#[from] ExtensionError), // Note that `from` here allows automatic conversion
+}

src/dfx-core/src/extension/manifest/custom_canister_type.rs

@@ -7,30 +7,34 @@ use serde::{Deserialize, Serialize};
 use serde_json::{Map as JsonMap, Value as JsonValue};
 use std::collections::BTreeMap;
 
-pub(crate) fn transform_dfx_json_via_extension(
-    json: &mut JsonValue,
-    extension_manager: ExtensionManager,
-) -> Result<(), ExtensionError> {
-    let canisters = match json.get_mut("canisters").and_then(|v| v.as_object_mut()) {
-        Some(canisters) => canisters,
-        None => return Ok(()),
-    };
-    for (canister_name, canister_declaration) in canisters.iter_mut() {
-        if let Some(canister_type) = get_valid_canister_type(canister_declaration) {
-            let canister_declaration = canister_declaration.as_object_mut().unwrap();
-            let (extension_name, canister_type) =
-                get_extension_name_and_custom_canister_type(&canister_type);
-            let extension_manifest = ExtensionManifest::get(extension_name, &extension_manager)?;
-            *canister_declaration = process_canister_declaration(
-                canister_declaration,
-                extension_name,
-                &extension_manifest,
-                canister_name,
-                canister_type,
-            )?;
+pub trait TransformConfiguration {
+    fn transform(&mut self, json: &mut serde_json::Value) -> Result<(), ExtensionError>;
+}
+
+impl TransformConfiguration for ExtensionManager {
+    fn transform(&mut self, json: &mut JsonValue) -> Result<(), ExtensionError> {
+        let canisters = match json.get_mut("canisters").and_then(|v| v.as_object_mut()) {
+            Some(canisters) => canisters,
+            None => return Ok(()),
+        };
+        for (canister_name, canister_declaration) in canisters.iter_mut() {
+            if let Some(canister_type) = get_valid_canister_type(canister_declaration) {
+                let canister_declaration = canister_declaration.as_object_mut().unwrap();
+                let (extension_name, canister_type) =
+                    get_extension_name_and_custom_canister_type(&canister_type);
+                let extension_manifest =
+                    ExtensionManifest::get_by_extension_name(extension_name, self)?;
+                *canister_declaration = process_canister_declaration(
+                    canister_declaration,
+                    extension_name,
+                    &extension_manifest,
+                    canister_name,
+                    canister_type,
+                )?;
+            }
         }
+        Ok(())
     }
-    Ok(())
 }
 
 fn get_valid_canister_type(canister_declaration: &mut JsonValue) -> Option<String> {
@@ -202,7 +206,7 @@ impl CustomCanisterTypeDeclaration {
 }
 
 #[cfg(test)]
-mod tests {
+mod custom_canister_type_declaration_tests {
     use super::*;
 
     macro_rules! test_op {
@@ -640,3 +644,13 @@ mod tests {
         );
     }
 }
+
+#[cfg(test)]
+pub struct NoopTransformConfiguration;
+#[cfg(test)]
+impl TransformConfiguration for NoopTransformConfiguration {
+    fn transform(&mut self, _: &mut serde_json::Value) -> Result<(), ExtensionError> {
+        // Do nothing
+        Ok(())
+    }
+}

src/dfx-core/src/extension/manifest/extension.rs

@@ -25,7 +25,7 @@ pub struct ExtensionManifest {
 }
 
 impl ExtensionManifest {
-    pub fn get(
+    pub fn get_by_extension_name(
         extension_name: &str,
         extension_manager: &ExtensionManager,
     ) -> Result<Self, ExtensionError> {

src/dfx-core/src/extension/mod.rs

@@ -33,7 +33,7 @@ impl Extension {
         self,
         manager: &ExtensionManager,
     ) -> Result<clap::Command, ExtensionError> {
-        let manifest = ExtensionManifest::get(&self.name, manager)?;
+        let manifest = ExtensionManifest::get_by_extension_name(&self.name, manager)?;
         let cmd = Command::new(&self.name)
             .bin_name(&self.name)
             // don't accept unknown options