From 6d2f688e8acaaccbb862d04f342d3947a3fa29be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 1 Aug 2023 12:56:46 +0100 Subject: [PATCH] Refactor agama-settings errors * They contain more information about the error. --- rust/agama-derive/src/lib.rs | 17 ++++++++++++----- rust/agama-lib/src/network/settings.rs | 7 ++++--- rust/agama-lib/src/storage/settings.rs | 6 +++--- rust/agama-lib/src/users/client.rs | 26 ++++++++++++++++++++++---- rust/agama-settings/src/error.rs | 20 +++++++++++++++++--- rust/agama-settings/src/settings.rs | 16 ++++++++-------- rust/agama-settings/tests/settings.rs | 13 +++++++------ 7 files changed, 73 insertions(+), 32 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index 2f3bed78da..090c6aea8f 100644 --- 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 index d9fdf3b18d..04316d95ba 100644 --- 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 for NetworkConnection { - type Error = SettingsError; + type Error = ConversionError; fn try_from(value: SettingObject) -> Result { 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 index ea64dce023..c6873edfa0 100644 --- 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 for Device { } impl TryFrom for Device { - type Error = SettingsError; + type Error = ConversionError; fn try_from(value: SettingObject) -> Result { 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 index cea370bee2..d510b20fd0 100644 --- 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 index 27afd229a6..b796c5b467 100644 --- 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 index fc00111443..9f1befb305 100644 --- 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> for SettingObject { } impl TryFrom for bool { - type Error = SettingsError; + type Error = ConversionError; fn try_from(value: SettingValue) -> Result { 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,7 +137,7 @@ impl TryFrom for bool { } impl TryFrom for Option { - type Error = SettingsError; + type Error = ConversionError; fn try_from(value: SettingValue) -> Result { Ok(Some(value.try_into()?)) @@ -145,7 +145,7 @@ impl TryFrom for Option { } impl TryFrom for String { - type Error = SettingsError; + type Error = ConversionError; fn try_from(value: SettingValue) -> Result { Ok(value.0) @@ -153,7 +153,7 @@ impl TryFrom for String { } impl TryFrom for Option { - type Error = SettingsError; + type Error = ConversionError; fn try_from(value: SettingValue) -> Result { Ok(Some(value.try_into()?)) @@ -175,7 +175,7 @@ mod tests { assert!(!value); let value = SettingValue("fasle".to_string()); - let value: Result = value.try_into(); + let value: Result = 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 index 974d36f5b5..9ed945444b 100644 --- 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 for Pattern { - type Error = SettingsError; + type Error = ConversionError; fn try_from(value: SettingObject) -> Result { 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" ); }