Refactor agama-settings errors

* They contain more information about the error.
agama-project · Aug 1, 2023 · 6d2f688 · 6d2f688
1 parent 3becbc2
commit 6d2f688
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 32 deletions.
diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs
@@ -84,7 +84,9 @@ fn expand_set_fn(settings: &SettingFieldsList) -> TokenStream2 {
         let field_name = scalar_fields.iter().map(|s| s.ident.clone());
         scalar_handling = quote! {
             match attr {
-                #(stringify!(#field_name) => self.#field_name = value.try_into()?,)*
+                #(stringify!(#field_name) => self.#field_name = value.try_into().map_err(|e| {
+                    agama_settings::SettingsError::UpdateFailed(attr.to_string(), e)
+                })?,)*
                 _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string()))
             }
         }
@@ -99,7 +101,7 @@ fn expand_set_fn(settings: &SettingFieldsList) -> TokenStream2 {
                 match ns {
                     #(stringify!(#field_name) => {
                         let #field_name = self.#field_name.get_or_insert(Default::default());
-                        #field_name.set(id, value)?
+                        #field_name.set(id, value).map_err(|e| e.with_attr(attr))?
                     })*
                     _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string()))
                 }
@@ -163,8 +165,13 @@ fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 {
         let field_name = collection_fields.iter().map(|s| s.ident.clone());
         collection_handling = quote! {
             match attr {
-                #(stringify!(#field_name) => self.#field_name.push(value.try_into()?),)*
-                _ => return Err(agama_settings::SettingsError::UnknownCollection(attr.to_string()))
+                #(stringify!(#field_name) => {
+                    let converted = value.try_into().map_err(|e| {
+                        agama_settings::SettingsError::UpdateFailed(attr.to_string(), e)
+                    })?;
+                    self.#field_name.push(converted)
+                },)*
+                _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string()))
             }
         };
     }
@@ -178,7 +185,7 @@ fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 {
                 match ns {
                     #(stringify!(#field_name) => {
                         let #field_name = self.#field_name.get_or_insert(Default::default());
-                        #field_name.add(id, value)?
+                        #field_name.add(id, value).map_err(|e| e.with_attr(attr))?
                     })*
                     _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string()))
                 }

diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs
@@ -1,7 +1,8 @@
 //! Representation of the network settings
 
 use super::types::DeviceType;
-use agama_settings::{SettingObject, SettingValue, Settings, SettingsError};
+use agama_settings::error::ConversionError;
+use agama_settings::{SettingObject, SettingValue, Settings};
 use serde::{Deserialize, Serialize};
 use std::convert::TryFrom;
 use std::default::Default;
@@ -54,11 +55,11 @@ impl NetworkConnection {
 }
 
 impl TryFrom<SettingObject> for NetworkConnection {
-    type Error = SettingsError;
+    type Error = ConversionError;
 
     fn try_from(value: SettingObject) -> Result<Self, Self::Error> {
         let Some(id) = value.get("id") else {
-            return Err(SettingsError::MissingKey("id".to_string()));
+            return Err(ConversionError::MissingKey("id".to_string()));
         };
 
         let default_method = SettingValue("disabled".to_string());

diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs
@@ -1,6 +1,6 @@
 //! Representation of the storage settings
 
-use agama_settings::{SettingObject, Settings, SettingsError};
+use agama_settings::{error::ConversionError, SettingObject, Settings};
 use serde::{Deserialize, Serialize};
 
 /// Storage settings for installation
@@ -31,14 +31,14 @@ impl From<String> for Device {
 }
 
 impl TryFrom<SettingObject> for Device {
-    type Error = SettingsError;
+    type Error = ConversionError;
 
     fn try_from(value: SettingObject) -> Result<Self, Self::Error> {
         match value.get("name") {
             Some(name) => Ok(Device {
                 name: name.clone().try_into()?,
             }),
-            _ => Err(SettingsError::MissingKey("name".to_string())),
+            _ => Err(ConversionError::MissingKey("name".to_string())),
         }
     }
 }
diff --git a/rust/agama-lib/src/users/client.rs b/rust/agama-lib/src/users/client.rs
@@ -42,13 +42,31 @@ impl FirstUser {
     }
 }
 
+// TODO: use the Settings macro (add support for ignoring fields to the macro and use Option for
+// FirstUser fields)
 impl Settings for FirstUser {
     fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), SettingsError> {
         match attr {
-            "full_name" => self.full_name = value.try_into()?,
-            "user_name" => self.user_name = value.try_into()?,
-            "password" => self.password = value.try_into()?,
-            "autologin" => self.autologin = value.try_into()?,
+            "full_name" => {
+                self.full_name = value
+                    .try_into()
+                    .map_err(|e| SettingsError::UpdateFailed(attr.to_string(), e))?
+            }
+            "user_name" => {
+                self.user_name = value
+                    .try_into()
+                    .map_err(|e| SettingsError::UpdateFailed(attr.to_string(), e))?
+            }
+            "password" => {
+                self.full_name = value
+                    .try_into()
+                    .map_err(|e| SettingsError::UpdateFailed(attr.to_string(), e))?
+            }
+            "autologin" => {
+                self.full_name = value
+                    .try_into()
+                    .map_err(|e| SettingsError::UpdateFailed(attr.to_string(), e))?
+            }
             _ => return Err(SettingsError::UnknownAttribute(attr.to_string())),
         }
         Ok(())

diff --git a/rust/agama-settings/src/error.rs b/rust/agama-settings/src/error.rs
@@ -4,10 +4,24 @@ use thiserror::Error;
 pub enum SettingsError {
     #[error("Unknown attribute '{0}'")]
     UnknownAttribute(String),
-    #[error("Unknown collection '{0}'")]
-    UnknownCollection(String),
+    #[error("Could not update '{0}': {1}")]
+    UpdateFailed(String, ConversionError),
+}
+
+#[derive(Error, Debug)]
+pub enum ConversionError {
     #[error("Invalid value '{0}', expected a {1}")]
-    InvalidValue(String, String), // TODO: add the value type name
+    InvalidValue(String, String),
     #[error("Missing key '{0}'")]
     MissingKey(String),
 }
+
+impl SettingsError {
+    /// Returns the an error with the updated attribute
+    pub fn with_attr(self, name: &str) -> Self {
+        match self {
+            Self::UnknownAttribute(_) => Self::UnknownAttribute(name.to_string()),
+            Self::UpdateFailed(_, source) => Self::UpdateFailed(name.to_string(), source),
+        }
+    }
+}
diff --git a/rust/agama-settings/src/settings.rs b/rust/agama-settings/src/settings.rs
@@ -12,7 +12,7 @@
 //! taking care of the conversions automatically. The newtype [SettingValue] takes care of such a
 //! conversion.
 //!
-use crate::error::SettingsError;
+use crate::error::{ConversionError, SettingsError};
 use std::collections::HashMap;
 /// For plain structs, the implementation can be derived.
 ///
@@ -60,7 +60,7 @@ use std::fmt::Display;
 /// ```
 pub trait Settings {
     fn add(&mut self, attr: &str, _value: SettingObject) -> Result<(), SettingsError> {
-        Err(SettingsError::UnknownCollection(attr.to_string()))
+        Err(SettingsError::UnknownAttribute(attr.to_string()))
     }
 
     fn set(&mut self, attr: &str, _value: SettingValue) -> Result<(), SettingsError> {
@@ -122,13 +122,13 @@ impl From<HashMap<String, String>> for SettingObject {
 }
 
 impl TryFrom<SettingValue> for bool {
-    type Error = SettingsError;
+    type Error = ConversionError;
 
     fn try_from(value: SettingValue) -> Result<Self, Self::Error> {
         match value.0.to_lowercase().as_str() {
             "true" | "yes" | "t" => Ok(true),
             "false" | "no" | "f" => Ok(false),
-            _ => Err(SettingsError::InvalidValue(
+            _ => Err(ConversionError::InvalidValue(
                 value.to_string(),
                 "boolean".to_string(),
             )),
@@ -137,23 +137,23 @@ impl TryFrom<SettingValue> for bool {
 }
 
 impl TryFrom<SettingValue> for Option<bool> {
-    type Error = SettingsError;
+    type Error = ConversionError;
 
     fn try_from(value: SettingValue) -> Result<Self, Self::Error> {
         Ok(Some(value.try_into()?))
     }
 }
 
 impl TryFrom<SettingValue> for String {
-    type Error = SettingsError;
+    type Error = ConversionError;
 
     fn try_from(value: SettingValue) -> Result<Self, Self::Error> {
         Ok(value.0)
     }
 }
 
 impl TryFrom<SettingValue> for Option<String> {
-    type Error = SettingsError;
+    type Error = ConversionError;
 
     fn try_from(value: SettingValue) -> Result<Self, Self::Error> {
         Ok(Some(value.try_into()?))
@@ -175,7 +175,7 @@ mod tests {
         assert!(!value);
 
         let value = SettingValue("fasle".to_string());
-        let value: Result<bool, SettingsError> = value.try_into();
+        let value: Result<bool, ConversionError> = value.try_into();
         let error = value.unwrap_err();
         assert_eq!(
             error.to_string(),

diff --git a/rust/agama-settings/tests/settings.rs b/rust/agama-settings/tests/settings.rs
@@ -1,5 +1,6 @@
-use agama_settings::settings::Settings;
-use agama_settings::{SettingObject, SettingValue, Settings, SettingsError};
+use agama_settings::{
+    error::ConversionError, settings::Settings, SettingObject, SettingValue, Settings,
+};
 use std::collections::HashMap;
 
 /// Main settings
@@ -26,14 +27,14 @@ pub struct Network {
 
 /// TODO: deriving this trait could be easy.
 impl TryFrom<SettingObject> for Pattern {
-    type Error = SettingsError;
+    type Error = ConversionError;
 
     fn try_from(value: SettingObject) -> Result<Self, Self::Error> {
         match value.get("id") {
             Some(id) => Ok(Pattern {
-                id: id.clone().try_into()?,
+                id: id.clone().to_string(),
             }),
-            _ => Err(SettingsError::MissingKey("id".to_string())),
+            _ => Err(ConversionError::MissingKey("id".to_string())),
         }
     }
 }
@@ -69,7 +70,7 @@ fn test_invalid_set() {
         .unwrap_err();
     assert_eq!(
         error.to_string(),
-        "Invalid value 'fasle', expected a boolean"
+        "Could not update 'network.enabled': Invalid value 'fasle', expected a boolean"
     );
 }