From 1bc91ecbf1ff46c1c9977d2d3950e852283c42c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Fri, 14 Jul 2023 13:48:14 +0100 Subject: [PATCH 01/18] Update to syn 2 --- rust/Cargo.lock | 2 +- rust/agama-derive/Cargo.toml | 4 ++-- rust/agama-derive/src/lib.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 745095d4c7..4d4483c4fe 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -50,7 +50,7 @@ version = "1.0.0" dependencies = [ "proc-macro2", "quote", - "syn 1.0.109", + "syn 2.0.16", ] [[package]] diff --git a/rust/agama-derive/Cargo.toml b/rust/agama-derive/Cargo.toml index e346846703..dcc7ab1160 100644 --- a/rust/agama-derive/Cargo.toml +++ b/rust/agama-derive/Cargo.toml @@ -9,5 +9,5 @@ proc-macro = true [dependencies] proc-macro2 = "1.0.51" -quote = "1.0.23" -syn = "1.0.107" +quote = "1.0" +syn = "2.0" diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index b44c9c4ffe..f435591502 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -19,7 +19,7 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { let (collection, scalar): (Vec<_>, Vec<_>) = fields.clone().into_iter().partition(|f| { f.attrs .iter() - .any(|a| a.path.is_ident("collection_setting")) + .any(|a| a.path().is_ident("collection_setting")) }); let scalar_field_names: Vec = From 421ab929dbc2e37576d5b37fb32619c2f2b6927e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 11 Apr 2023 15:39:32 +0100 Subject: [PATCH 02/18] Replace 'collection_setting' with 'settings(collection)' --- rust/agama-derive/src/lib.rs | 83 +++++++++++++++++++------- rust/agama-lib/src/storage/settings.rs | 2 +- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index f435591502..12f1a7b5cf 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -1,11 +1,23 @@ use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; use quote::quote; -use syn::{parse_macro_input, DeriveInput, Fields, Ident}; +use syn::{parse_macro_input, DeriveInput, Fields}; + +#[derive(Debug, Clone)] +enum SettingKind { + Scalar, + Collection, +} + +#[derive(Debug, Clone)] +struct SettingField { + ident: syn::Ident, + kind: SettingKind, +} /// Derive Settings, typically for a FooSettings struct. /// (see agama_lib::settings::Settings but I cannot link to it without a circular dependency) -#[proc_macro_derive(Settings, attributes(collection_setting))] +#[proc_macro_derive(Settings, attributes(settings))] pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); let fields = match &input.data { @@ -16,23 +28,24 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { _ => panic!("only structs are supported"), }; - let (collection, scalar): (Vec<_>, Vec<_>) = fields.clone().into_iter().partition(|f| { - f.attrs - .iter() - .any(|a| a.path().is_ident("collection_setting")) - }); + let fields: Vec<&syn::Field> = fields.iter().collect(); + let settings = parse_setting_fields(fields); - let scalar_field_names: Vec = - scalar.into_iter().filter_map(|field| field.ident).collect(); + let scalar_fields: Vec = settings + .clone() + .into_iter() + .filter(|s| matches!(s.kind, SettingKind::Scalar)) + .collect(); - let set_fn = expand_set_fn(&scalar_field_names); - let merge_fn = expand_merge_fn(&scalar_field_names); + let set_fn = expand_set_fn(&scalar_fields); + let merge_fn = expand_merge_fn(&scalar_fields); - let collection_field_names: Vec = collection + let collection_fields: Vec = settings + .clone() .into_iter() - .filter_map(|field| field.ident) + .filter(|s| matches!(s.kind, SettingKind::Collection)) .collect(); - let add_fn = expand_add_fn(&collection_field_names); + let add_fn = expand_add_fn(&collection_fields); let name = input.ident; let expanded = quote! { @@ -46,11 +59,12 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { expanded.into() } -fn expand_set_fn(field_name: &Vec) -> TokenStream2 { - if field_name.is_empty() { +fn expand_set_fn(settings: &Vec) -> TokenStream2 { + if settings.is_empty() { return quote! {}; } + let field_name = settings.iter().map(|s| s.ident.clone()); quote! { fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), crate::settings::SettingsError> { match attr { @@ -62,11 +76,12 @@ fn expand_set_fn(field_name: &Vec) -> TokenStream2 { } } -fn expand_merge_fn(field_name: &Vec) -> TokenStream2 { - if field_name.is_empty() { +fn expand_merge_fn(settings: &Vec) -> TokenStream2 { + if settings.is_empty() { return quote! {}; } + let field_name = settings.iter().map(|s| s.ident.clone()); quote! { fn merge(&mut self, other: &Self) where @@ -79,11 +94,12 @@ fn expand_merge_fn(field_name: &Vec) -> TokenStream2 { } } -fn expand_add_fn(field_name: &Vec) -> TokenStream2 { - if field_name.is_empty() { +fn expand_add_fn(settings: &Vec) -> TokenStream2 { + if settings.is_empty() { return quote! {}; } + let field_name = settings.iter().map(|s| s.ident.clone()); quote! { fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), crate::settings::SettingsError> { match attr { @@ -94,3 +110,30 @@ fn expand_add_fn(field_name: &Vec) -> TokenStream2 { } } } + +// Extracts information about the settings fields +fn parse_setting_fields(fields: Vec<&syn::Field>) -> Vec { + let mut settings = vec![]; + for field in fields { + let mut setting = SettingField { + ident: field.ident.clone().expect("could not find an ident"), + kind: SettingKind::Scalar, + }; + + for attr in &field.attrs { + if !attr.path().is_ident("settings") { + continue; + } + attr.parse_nested_meta(|meta| { + if meta.path.is_ident("collection") { + setting.kind = SettingKind::Collection; + }; + + Ok(()) + }) + .expect("wrong arguments to the settings attribute"); + } + settings.push(setting); + } + settings +} diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs index cdba404196..a79b375713 100644 --- a/rust/agama-lib/src/storage/settings.rs +++ b/rust/agama-lib/src/storage/settings.rs @@ -13,7 +13,7 @@ pub struct StorageSettings { /// Encryption password for the storage devices (in clear text) pub encryption_password: Option, /// Devices to use in the installation - #[collection_setting] + #[settings(collection)] pub devices: Vec, } From 8ebcca282d1ccea7f6a3e16b6a169506f3e4e9ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 11 Apr 2023 16:02:21 +0100 Subject: [PATCH 03/18] Refactor the filtering code of the Settings macro --- rust/agama-derive/src/lib.rs | 45 +++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index 12f1a7b5cf..4f87dfaeb1 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -3,16 +3,28 @@ use proc_macro2::TokenStream as TokenStream2; use quote::quote; use syn::{parse_macro_input, DeriveInput, Fields}; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy, PartialEq)] enum SettingKind { Scalar, Collection, } +/// Represents a setting and its configuration #[derive(Debug, Clone)] struct SettingField { - ident: syn::Ident, - kind: SettingKind, + /// Setting ident + pub ident: syn::Ident, + /// Setting kind (scalar, collection, struct). + pub kind: SettingKind, +} + +/// List of setting fields +struct SettingFieldsList(Vec); + +impl SettingFieldsList { + pub fn by_type(&self, kind: SettingKind) -> Vec<&SettingField> { + self.0.iter().filter(|f| f.kind == kind).collect() + } } /// Derive Settings, typically for a FooSettings struct. @@ -31,20 +43,11 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { let fields: Vec<&syn::Field> = fields.iter().collect(); let settings = parse_setting_fields(fields); - let scalar_fields: Vec = settings - .clone() - .into_iter() - .filter(|s| matches!(s.kind, SettingKind::Scalar)) - .collect(); - + let scalar_fields = settings.by_type(SettingKind::Scalar); let set_fn = expand_set_fn(&scalar_fields); let merge_fn = expand_merge_fn(&scalar_fields); - let collection_fields: Vec = settings - .clone() - .into_iter() - .filter(|s| matches!(s.kind, SettingKind::Collection)) - .collect(); + let collection_fields = settings.by_type(SettingKind::Collection); let add_fn = expand_add_fn(&collection_fields); let name = input.ident; @@ -59,7 +62,7 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { expanded.into() } -fn expand_set_fn(settings: &Vec) -> TokenStream2 { +fn expand_set_fn(settings: &Vec<&SettingField>) -> TokenStream2 { if settings.is_empty() { return quote! {}; } @@ -76,7 +79,7 @@ fn expand_set_fn(settings: &Vec) -> TokenStream2 { } } -fn expand_merge_fn(settings: &Vec) -> TokenStream2 { +fn expand_merge_fn(settings: &Vec<&SettingField>) -> TokenStream2 { if settings.is_empty() { return quote! {}; } @@ -94,7 +97,7 @@ fn expand_merge_fn(settings: &Vec) -> TokenStream2 { } } -fn expand_add_fn(settings: &Vec) -> TokenStream2 { +fn expand_add_fn(settings: &Vec<&SettingField>) -> TokenStream2 { if settings.is_empty() { return quote! {}; } @@ -112,11 +115,11 @@ fn expand_add_fn(settings: &Vec) -> TokenStream2 { } // Extracts information about the settings fields -fn parse_setting_fields(fields: Vec<&syn::Field>) -> Vec { +fn parse_setting_fields(fields: Vec<&syn::Field>) -> SettingFieldsList { let mut settings = vec![]; for field in fields { let mut setting = SettingField { - ident: field.ident.clone().expect("could not find an ident"), + ident: field.ident.clone().expect("to find a field ident"), kind: SettingKind::Scalar, }; @@ -131,9 +134,9 @@ fn parse_setting_fields(fields: Vec<&syn::Field>) -> Vec { Ok(()) }) - .expect("wrong arguments to the settings attribute"); + .expect("settings arguments do not follow the expected structure"); } settings.push(setting); } - settings + SettingFieldsList(settings) } From e5caaacabb8b5bf02ec5cda84e24386ac992638f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Sun, 16 Jul 2023 10:29:03 +0100 Subject: [PATCH 04/18] Extend settings derive to support nested and collection fields --- rust/agama-derive/src/lib.rs | 130 ++++++++++++++++++++++++++++------- 1 file changed, 106 insertions(+), 24 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index 4f87dfaeb1..a31bb8b073 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -7,6 +7,7 @@ use syn::{parse_macro_input, DeriveInput, Fields}; enum SettingKind { Scalar, Collection, + Nested, } /// Represents a setting and its configuration @@ -19,12 +20,22 @@ struct SettingField { } /// List of setting fields +#[derive(Debug)] struct SettingFieldsList(Vec); impl SettingFieldsList { pub fn by_type(&self, kind: SettingKind) -> Vec<&SettingField> { self.0.iter().filter(|f| f.kind == kind).collect() } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + // TODO: implement Iterator? + pub fn all(&self) -> &Vec { + &self.0 + } } /// Derive Settings, typically for a FooSettings struct. @@ -43,12 +54,9 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { let fields: Vec<&syn::Field> = fields.iter().collect(); let settings = parse_setting_fields(fields); - let scalar_fields = settings.by_type(SettingKind::Scalar); - let set_fn = expand_set_fn(&scalar_fields); - let merge_fn = expand_merge_fn(&scalar_fields); - - let collection_fields = settings.by_type(SettingKind::Collection); - let add_fn = expand_add_fn(&collection_fields); + let set_fn = expand_set_fn(&settings); + let add_fn = expand_add_fn(&settings); + let merge_fn = expand_merge_fn(&settings); let name = input.ident; let expanded = quote! { @@ -62,53 +70,123 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { expanded.into() } -fn expand_set_fn(settings: &Vec<&SettingField>) -> TokenStream2 { +fn expand_set_fn(settings: &SettingFieldsList) -> TokenStream2 { if settings.is_empty() { return quote! {}; } - let field_name = settings.iter().map(|s| s.ident.clone()); - quote! { - fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), crate::settings::SettingsError> { + let mut scalar_handling = quote! {}; + let scalar_fields = settings.by_type(SettingKind::Scalar); + if !scalar_fields.is_empty() { + 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()?,)* - _ => return Err(SettingsError::UnknownAttribute(attr.to_string())) - }; - Ok(()) + _ => return Err(crate::settings::SettingsError::UnknownAttribute(attr.to_string())) + } + } + } + + let mut nested_handling = quote! {}; + let nested_fields = settings.by_type(SettingKind::Nested); + if !nested_fields.is_empty() { + let field_name = nested_fields.iter().map(|s| s.ident.clone()); + nested_handling = quote! { + if let Some((ns, id)) = attr.split_once('.') { + match ns { + #(stringify!(#field_name) => { + let #field_name = self.#field_name.get_or_insert(Default::default()); + #field_name.set(id, value)? + })* + _ => return Err(crate::settings::SettingsError::UnknownAttribute(attr.to_string())) + } + return Ok(()) + } } } + + quote! { + fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), crate::settings::SettingsError> { + #nested_handling + #scalar_handling + Ok(()) + } + } } -fn expand_merge_fn(settings: &Vec<&SettingField>) -> TokenStream2 { +fn expand_merge_fn(settings: &SettingFieldsList) -> TokenStream2 { if settings.is_empty() { return quote! {}; } - let field_name = settings.iter().map(|s| s.ident.clone()); + let arms = settings.all().iter().map(|s| { + let field_name = &s.ident; + match s.kind { + SettingKind::Scalar => quote! { + if let Some(value) = &other.#field_name { + self.#field_name = Some(value.clone()) + } + }, + SettingKind::Nested => quote! { + if let Some(other_value) = &other.#field_name { + let nested = self.#field_name.get_or_insert(Default::default()); + nested.merge(other_value); + } + }, + SettingKind::Collection => quote! { + self.#field_name = other.#field_name.clone(); + }, + } + }); + quote! { fn merge(&mut self, other: &Self) where Self: Sized, { - #(if let Some(value) = &other.#field_name { - self.#field_name = Some(value.clone()) - })* + #(#arms)* } } } -fn expand_add_fn(settings: &Vec<&SettingField>) -> TokenStream2 { +fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 { if settings.is_empty() { return quote! {}; } - let field_name = settings.iter().map(|s| s.ident.clone()); - quote! { - fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), crate::settings::SettingsError> { + let mut collection_handling = quote! {}; + let collection_fields = settings.by_type(SettingKind::Collection); + if !collection_fields.is_empty() { + 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(SettingsError::UnknownCollection(attr.to_string())) - }; + _ => return Err(crate::settings::SettingsError::UnknownCollection(attr.to_string())) + } + }; + } + + let mut nested_handling = quote! {}; + let nested_fields = settings.by_type(SettingKind::Nested); + if !nested_fields.is_empty() { + let field_name = nested_fields.iter().map(|s| s.ident.clone()); + nested_handling = quote! { + if let Some((ns, id)) = attr.split_once('.') { + match ns { + #(stringify!(#field_name) => { + let #field_name = self.#field_name.get_or_insert(Default::default()); + #field_name.add(id, value)? + })* + _ => return Err(crate::settings::SettingsError::UnknownAttribute(attr.to_string())) + } + } + } + } + + quote! { + fn add(&mut self, attr: &str, value: crate::settings::SettingObject) -> Result<(), crate::settings::SettingsError> { + #nested_handling + #collection_handling Ok(()) } } @@ -132,6 +210,10 @@ fn parse_setting_fields(fields: Vec<&syn::Field>) -> SettingFieldsList { setting.kind = SettingKind::Collection; }; + if meta.path.is_ident("nested") { + setting.kind = SettingKind::Nested; + } + Ok(()) }) .expect("settings arguments do not follow the expected structure"); From 83d8018d901ce18e43cbe10ba7a532aba818dbb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Sun, 16 Jul 2023 10:32:20 +0100 Subject: [PATCH 05/18] Derive Settings for install and network settings --- rust/agama-lib/src/install_settings.rs | 89 ++----------------------- rust/agama-lib/src/network/settings.rs | 21 +----- rust/agama-lib/src/software/settings.rs | 2 +- rust/agama-lib/src/storage/settings.rs | 2 +- rust/agama-lib/src/users/settings.rs | 37 ++-------- 5 files changed, 16 insertions(+), 135 deletions(-) diff --git a/rust/agama-lib/src/install_settings.rs b/rust/agama-lib/src/install_settings.rs index 9e5f0285d6..a39b92c086 100644 --- a/rust/agama-lib/src/install_settings.rs +++ b/rust/agama-lib/src/install_settings.rs @@ -1,11 +1,12 @@ //! Configuration settings handling //! //! This module implements the mechanisms to load and store the installation settings. -use crate::settings::{SettingObject, SettingValue, Settings, SettingsError}; +use crate::settings::Settings; use crate::{ network::NetworkSettings, software::SoftwareSettings, storage::StorageSettings, users::UserSettings, }; +use agama_derive::Settings; use serde::{Deserialize, Serialize}; use std::default::Default; use std::str::FromStr; @@ -58,16 +59,20 @@ impl FromStr for Scope { /// /// This struct represents installation settings. It serves as an entry point and it is composed of /// other structs which hold the settings for each area ("users", "software", etc.). -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Settings, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct InstallSettings { #[serde(default, flatten)] + #[settings(nested)] pub user: Option, #[serde(default)] + #[settings(nested)] pub software: Option, #[serde(default)] + #[settings(nested)] pub storage: Option, #[serde(default)] + #[settings(nested)] pub network: Option, } @@ -91,83 +96,3 @@ impl InstallSettings { scopes } } - -impl Settings for InstallSettings { - fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), SettingsError> { - if let Some((ns, id)) = attr.split_once('.') { - match ns { - "network" => { - let network = self.network.get_or_insert(Default::default()); - network.add(id, value)? - } - "software" => { - let software = self.software.get_or_insert(Default::default()); - software.add(id, value)? - } - "user" => { - let user = self.user.get_or_insert(Default::default()); - user.add(id, value)? - } - "storage" => { - let storage = self.storage.get_or_insert(Default::default()); - storage.add(id, value)? - } - _ => return Err(SettingsError::UnknownCollection(attr.to_string())), - } - } - Ok(()) - } - - fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), SettingsError> { - if let Some((ns, id)) = attr.split_once('.') { - match ns { - "network" => { - let network = self.network.get_or_insert(Default::default()); - network.set(id, value)? - } - "software" => { - let software = self.software.get_or_insert(Default::default()); - software.set(id, value)? - } - "user" => { - let user = self.user.get_or_insert(Default::default()); - // User settings are flatten. Pass the full attribute name. - user.set(attr, value)? - } - "root" => { - let root = self.user.get_or_insert(Default::default()); - // Root settings are flatten. Pass the full attribute name. - root.set(attr, value)? - } - "storage" => { - let storage = self.storage.get_or_insert(Default::default()); - storage.set(id, value)? - } - _ => return Err(SettingsError::UnknownAttribute(attr.to_string())), - } - } - Ok(()) - } - - fn merge(&mut self, other: &Self) { - if let Some(other_network) = &other.network { - let network = self.network.get_or_insert(Default::default()); - network.merge(other_network); - } - - if let Some(other_software) = &other.software { - let software = self.software.get_or_insert(Default::default()); - software.merge(other_software); - } - - if let Some(other_user) = &other.user { - let user = self.user.get_or_insert(Default::default()); - user.merge(other_user); - } - - if let Some(other_storage) = &other.storage { - let storage = self.storage.get_or_insert(Default::default()); - storage.merge(other_storage); - } - } -} diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index c044173346..afae94a7ee 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -2,35 +2,20 @@ use super::types::DeviceType; use crate::settings::{SettingObject, SettingValue, Settings, SettingsError}; +use agama_derive::Settings; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::default::Default; /// Network settings for installation -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Settings, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct NetworkSettings { /// Connections to use in the installation + #[settings(collection)] pub connections: Vec, } -impl Settings for NetworkSettings { - fn add(&mut self, attr: &str, value: SettingObject) -> Result<(), SettingsError> { - match attr { - "connections" => self.connections.push(value.try_into()?), - _ => return Err(SettingsError::UnknownAttribute(attr.to_string())), - }; - Ok(()) - } - - fn merge(&mut self, other: &Self) - where - Self: Sized, - { - self.connections = other.connections.clone(); - } -} - #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct WirelessSettings { #[serde(skip_serializing_if = "String::is_empty")] diff --git a/rust/agama-lib/src/software/settings.rs b/rust/agama-lib/src/software/settings.rs index c345aea578..776e414312 100644 --- a/rust/agama-lib/src/software/settings.rs +++ b/rust/agama-lib/src/software/settings.rs @@ -1,6 +1,6 @@ //! Representation of the software settings -use crate::settings::{Settings, SettingsError}; +use crate::settings::Settings; use agama_derive::Settings; use serde::{Deserialize, Serialize}; diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs index a79b375713..591282cb5a 100644 --- a/rust/agama-lib/src/storage/settings.rs +++ b/rust/agama-lib/src/storage/settings.rs @@ -18,7 +18,7 @@ pub struct StorageSettings { } /// Device to use in the installation -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct Device { /// Device name (e.g., "/dev/sda") diff --git a/rust/agama-lib/src/users/settings.rs b/rust/agama-lib/src/users/settings.rs index b7eff8aa5a..a64674c868 100644 --- a/rust/agama-lib/src/users/settings.rs +++ b/rust/agama-lib/src/users/settings.rs @@ -1,49 +1,20 @@ -use crate::settings::{SettingValue, Settings, SettingsError}; +use crate::settings::Settings; use agama_derive::Settings; use serde::{Deserialize, Serialize}; /// User settings /// /// Holds the user settings for the installation. -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Settings, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct UserSettings { #[serde(rename = "user")] + #[settings(nested)] pub first_user: Option, + #[settings(nested)] pub root: Option, } -impl Settings for UserSettings { - fn set(&mut self, attr: &str, value: SettingValue) -> Result<(), SettingsError> { - if let Some((ns, id)) = attr.split_once('.') { - match ns { - "user" => { - let first_user = self.first_user.get_or_insert(Default::default()); - first_user.set(id, value)? - } - "root" => { - let root_user = self.root.get_or_insert(Default::default()); - root_user.set(id, value)? - } - _ => return Err(SettingsError::UnknownAttribute(attr.to_string())), - } - } - Ok(()) - } - - fn merge(&mut self, other: &Self) { - if let Some(other_first_user) = &other.first_user { - let first_user = self.first_user.get_or_insert(Default::default()); - first_user.merge(other_first_user); - } - - if let Some(other_root) = &other.root { - let root = self.root.get_or_insert(Default::default()); - root.merge(other_root); - } - } -} - /// First user settings /// /// Holds the settings for the first user. From da6a1be1b7ff3c5b36bf7a774963633ab17799dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 18 Jul 2023 15:38:22 +0100 Subject: [PATCH 06/18] Move the Settings trait to a separate package --- rust/Cargo.lock | 54 +++++++++++-------- rust/Cargo.toml | 5 +- rust/agama-cli/Cargo.toml | 1 + rust/agama-cli/src/config.rs | 2 +- rust/agama-derive/src/lib.rs | 12 ++--- rust/agama-lib/Cargo.toml | 1 + rust/agama-lib/src/install_settings.rs | 2 +- rust/agama-lib/src/lib.rs | 1 - rust/agama-lib/src/network/settings.rs | 4 +- rust/agama-lib/src/software/settings.rs | 2 +- rust/agama-lib/src/storage/settings.rs | 2 +- rust/agama-lib/src/users/client.rs | 2 +- rust/agama-lib/src/users/settings.rs | 2 +- rust/agama-settings/Cargo.toml | 10 ++++ rust/agama-settings/src/error.rs | 13 +++++ rust/agama-settings/src/lib.rs | 5 ++ .../src/settings.rs | 18 ++----- 17 files changed, 82 insertions(+), 54 deletions(-) create mode 100644 rust/agama-settings/Cargo.toml create mode 100644 rust/agama-settings/src/error.rs create mode 100644 rust/agama-settings/src/lib.rs rename rust/{agama-lib => agama-settings}/src/settings.rs (92%) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 4d4483c4fe..9f25fad680 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,6 +13,7 @@ name = "agama-cli" version = "1.0.0" dependencies = [ "agama-lib", + "agama-settings", "anyhow", "async-std", "clap", @@ -50,7 +51,7 @@ version = "1.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -58,6 +59,7 @@ name = "agama-lib" version = "1.0.0" dependencies = [ "agama-derive", + "agama-settings", "anyhow", "async-std", "curl", @@ -84,6 +86,14 @@ dependencies = [ "serde", ] +[[package]] +name = "agama-settings" +version = "1.0.0" +dependencies = [ + "agama-derive", + "thiserror", +] + [[package]] name = "ahash" version = "0.8.3" @@ -288,7 +298,7 @@ checksum = "0e97ce7de6cf12de5d7226c73f5ba9811622f4db3a5b91b55c53e987e5f91cba" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -332,7 +342,7 @@ checksum = "b9ccdd8f2a161be9bd5c023df56f1b2a0bd1d83872ae53b71a84a12c9bf6e842" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -493,7 +503,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -662,7 +672,7 @@ checksum = "5e9a1f9f7d83e59740248a6e14ecf93929ade55027844dfcea78beafccc15745" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -811,7 +821,7 @@ checksum = "89ca545a94061b6365f2c7355b4b32bd20df3ff95f02da9329b34ccc3bd6ee72" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -1435,9 +1445,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.58" +version = "1.0.66" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa1fb82fc0c281dd9671101b66b771ebbe1eaf967b96ac8740dcba4b70005ca8" +checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9" dependencies = [ "unicode-ident", ] @@ -1454,9 +1464,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.27" +version = "1.0.31" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f4f29d145265ec1c483c7c654450edde0bfe043d3938d6972630663356d9500" +checksum = "5fe8a65d69dd0808184ebb5f836ab526bb259db23c657efa38711b1072ee47f0" dependencies = [ "proc-macro2", ] @@ -1578,7 +1588,7 @@ checksum = "8c805777e3930c8883389c602315a24224bcc738b63905ef87cd1420353ea93e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -1600,7 +1610,7 @@ checksum = "bcec881020c684085e55a25f7fd888954d56609ef363479dc5a1305eb0d40cab" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -1730,9 +1740,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.16" +version = "2.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a6f671d4b5ffdb8eadec19c0ae67fe2639df8684bd7bc4b83d986b8db549cf01" +checksum = "45c3457aacde3c65315de5031ec191ce46604304d2446e803d71ade03308d970" dependencies = [ "proc-macro2", "quote", @@ -1773,22 +1783,22 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.40" +version = "1.0.43" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "978c9a314bd8dc99be594bc3c175faaa9794be04a5a5e153caba6915336cebac" +checksum = "a35fc5b8971143ca348fa6df4f024d4d55264f3468c71ad1c2f365b0a4d58c42" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.40" +version = "1.0.43" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9456a42c5b0d803c8cd86e73dd7cc9edd429499f37a3550d286d5e86720569f" +checksum = "463fe12d7993d3b327787537ce8dd4dfa058de32fc2b195ef3cde03dc4771e8f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -1872,7 +1882,7 @@ checksum = "0f57e3ca2a01450b1a921183a9c9cbfda207fd822cef4ccb00a65402cbba7a74" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", ] [[package]] @@ -2021,7 +2031,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", "wasm-bindgen-shared", ] @@ -2055,7 +2065,7 @@ checksum = "e128beba882dd1eb6200e1dc92ae6c5dbaa4311aa7bb211ca035779e5efc39f8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.16", + "syn 2.0.26", "wasm-bindgen-backend", "wasm-bindgen-shared", ] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 0015478bee..002eba41ec 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -4,5 +4,6 @@ members = [ "agama-dbus-server", "agama-derive", "agama-lib", - "agama-locale-data" -] \ No newline at end of file + "agama-locale-data", + "agama-settings" +] diff --git a/rust/agama-cli/Cargo.toml b/rust/agama-cli/Cargo.toml index 38765fee9a..fcf9039578 100644 --- a/rust/agama-cli/Cargo.toml +++ b/rust/agama-cli/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" [dependencies] clap = { version = "4.1.4", features = ["derive"] } agama-lib = { path="../agama-lib" } +agama-settings = { path="../agama-settings" } serde = { version = "1.0.152" } serde_json = "1.0.91" serde_yaml = "0.9.17" diff --git a/rust/agama-cli/src/config.rs b/rust/agama-cli/src/config.rs index ce517106f9..d71d43f31d 100644 --- a/rust/agama-cli/src/config.rs +++ b/rust/agama-cli/src/config.rs @@ -2,8 +2,8 @@ use crate::error::CliError; use crate::printers::{print, Format}; use agama_lib::connection; use agama_lib::install_settings::{InstallSettings, Scope}; -use agama_lib::settings::{SettingObject, SettingValue, Settings}; use agama_lib::Store as SettingsStore; +use agama_settings::{settings::Settings, SettingObject, SettingValue}; use clap::Subcommand; use convert_case::{Case, Casing}; use std::str::FromStr; diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index a31bb8b073..09512dac40 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -82,7 +82,7 @@ fn expand_set_fn(settings: &SettingFieldsList) -> TokenStream2 { scalar_handling = quote! { match attr { #(stringify!(#field_name) => self.#field_name = value.try_into()?,)* - _ => return Err(crate::settings::SettingsError::UnknownAttribute(attr.to_string())) + _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string())) } } } @@ -98,7 +98,7 @@ fn expand_set_fn(settings: &SettingFieldsList) -> TokenStream2 { let #field_name = self.#field_name.get_or_insert(Default::default()); #field_name.set(id, value)? })* - _ => return Err(crate::settings::SettingsError::UnknownAttribute(attr.to_string())) + _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string())) } return Ok(()) } @@ -106,7 +106,7 @@ fn expand_set_fn(settings: &SettingFieldsList) -> TokenStream2 { } quote! { - fn set(&mut self, attr: &str, value: crate::settings::SettingValue) -> Result<(), crate::settings::SettingsError> { + fn set(&mut self, attr: &str, value: agama_settings::SettingValue) -> Result<(), agama_settings::SettingsError> { #nested_handling #scalar_handling Ok(()) @@ -161,7 +161,7 @@ fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 { collection_handling = quote! { match attr { #(stringify!(#field_name) => self.#field_name.push(value.try_into()?),)* - _ => return Err(crate::settings::SettingsError::UnknownCollection(attr.to_string())) + _ => return Err(agama_settings::SettingsError::UnknownCollection(attr.to_string())) } }; } @@ -177,14 +177,14 @@ fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 { let #field_name = self.#field_name.get_or_insert(Default::default()); #field_name.add(id, value)? })* - _ => return Err(crate::settings::SettingsError::UnknownAttribute(attr.to_string())) + _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string())) } } } } quote! { - fn add(&mut self, attr: &str, value: crate::settings::SettingObject) -> Result<(), crate::settings::SettingsError> { + fn add(&mut self, attr: &str, value: agama_settings::SettingObject) -> Result<(), agama_settings::SettingsError> { #nested_handling #collection_handling Ok(()) diff --git a/rust/agama-lib/Cargo.toml b/rust/agama-lib/Cargo.toml index 2154f67820..cf63829e25 100644 --- a/rust/agama-lib/Cargo.toml +++ b/rust/agama-lib/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] agama-derive = { path="../agama-derive" } +agama-settings = { path="../agama-settings" } anyhow = "1.0" async-std = "1.12.0" curl = { version = "0.4.44", features = ["protocol-ftp"] } diff --git a/rust/agama-lib/src/install_settings.rs b/rust/agama-lib/src/install_settings.rs index a39b92c086..27c641a96a 100644 --- a/rust/agama-lib/src/install_settings.rs +++ b/rust/agama-lib/src/install_settings.rs @@ -1,7 +1,7 @@ //! Configuration settings handling //! //! This module implements the mechanisms to load and store the installation settings. -use crate::settings::Settings; +use agama_settings::settings::Settings; use crate::{ network::NetworkSettings, software::SoftwareSettings, storage::StorageSettings, users::UserSettings, diff --git a/rust/agama-lib/src/lib.rs b/rust/agama-lib/src/lib.rs index b6b6ec2122..5436218f9b 100644 --- a/rust/agama-lib/src/lib.rs +++ b/rust/agama-lib/src/lib.rs @@ -28,7 +28,6 @@ pub mod install_settings; pub mod manager; pub mod network; pub mod profile; -pub mod settings; pub mod software; pub mod storage; pub mod users; diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index afae94a7ee..adf4deecb7 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -1,7 +1,7 @@ //! Representation of the network settings use super::types::DeviceType; -use crate::settings::{SettingObject, SettingValue, Settings, SettingsError}; +use agama_settings::{SettingsError, SettingObject, SettingValue, settings::Settings}; use agama_derive::Settings; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; @@ -78,7 +78,7 @@ impl TryFrom for NetworkConnection { #[cfg(test)] mod tests { use super::*; - use crate::settings::{SettingObject, SettingValue}; + use agama_settings::settings::{SettingObject, SettingValue}; use std::collections::HashMap; #[test] diff --git a/rust/agama-lib/src/software/settings.rs b/rust/agama-lib/src/software/settings.rs index 776e414312..bb85c61136 100644 --- a/rust/agama-lib/src/software/settings.rs +++ b/rust/agama-lib/src/software/settings.rs @@ -1,6 +1,6 @@ //! Representation of the software settings -use crate::settings::Settings; +use agama_settings::settings::Settings; use agama_derive::Settings; use serde::{Deserialize, Serialize}; diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs index 591282cb5a..794c3f5641 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 crate::settings::{SettingObject, Settings, SettingsError}; +use agama_settings::{SettingsError, SettingObject, settings::Settings}; use agama_derive::Settings; use serde::{Deserialize, Serialize}; diff --git a/rust/agama-lib/src/users/client.rs b/rust/agama-lib/src/users/client.rs index 0c22a5e398..cea370bee2 100644 --- a/rust/agama-lib/src/users/client.rs +++ b/rust/agama-lib/src/users/client.rs @@ -2,7 +2,7 @@ use super::proxies::Users1Proxy; use crate::error::ServiceError; -use crate::settings::{SettingValue, Settings, SettingsError}; +use agama_settings::{settings::Settings, SettingValue, SettingsError}; use serde::Serialize; use zbus::Connection; diff --git a/rust/agama-lib/src/users/settings.rs b/rust/agama-lib/src/users/settings.rs index a64674c868..783f10f4fd 100644 --- a/rust/agama-lib/src/users/settings.rs +++ b/rust/agama-lib/src/users/settings.rs @@ -1,4 +1,4 @@ -use crate::settings::Settings; +use agama_settings::settings::Settings; use agama_derive::Settings; use serde::{Deserialize, Serialize}; diff --git a/rust/agama-settings/Cargo.toml b/rust/agama-settings/Cargo.toml new file mode 100644 index 0000000000..45ebf2bacc --- /dev/null +++ b/rust/agama-settings/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "agama-settings" +version = "1.0.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +agama-derive = { path="../agama-derive" } +thiserror = "1.0.43" diff --git a/rust/agama-settings/src/error.rs b/rust/agama-settings/src/error.rs new file mode 100644 index 0000000000..eb960e4e3b --- /dev/null +++ b/rust/agama-settings/src/error.rs @@ -0,0 +1,13 @@ +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum SettingsError { + #[error("Unknown attribute '{0}'")] + UnknownAttribute(String), + #[error("Unknown collection '{0}'")] + UnknownCollection(String), + #[error("Invalid value '{0}'")] + InvalidValue(String), + #[error("Missing key '{0}'")] + MissingKey(String), +} diff --git a/rust/agama-settings/src/lib.rs b/rust/agama-settings/src/lib.rs new file mode 100644 index 0000000000..172afabe3a --- /dev/null +++ b/rust/agama-settings/src/lib.rs @@ -0,0 +1,5 @@ +pub mod error; +pub mod settings; + +pub use self::error::SettingsError; +pub use self::settings::{SettingObject, SettingValue}; \ No newline at end of file diff --git a/rust/agama-lib/src/settings.rs b/rust/agama-settings/src/settings.rs similarity index 92% rename from rust/agama-lib/src/settings.rs rename to rust/agama-settings/src/settings.rs index 29af38a533..302b749651 100644 --- a/rust/agama-lib/src/settings.rs +++ b/rust/agama-settings/src/settings.rs @@ -12,13 +12,13 @@ //! taking care of the conversions automatically. The newtype [SettingValue] takes care of such a //! conversion. //! +use crate::error::SettingsError; use std::collections::HashMap; /// For plain structs, the implementation can be derived. /// /// TODO: derive for top-level structs too use std::convert::TryFrom; use std::fmt::Display; -use thiserror::Error; /// Implements support for easily settings attributes values given an ID (`"users.name"`) and a /// string value (`"Foo bar"`). @@ -27,7 +27,7 @@ use thiserror::Error; /// `UserSettings`. /// /// ```no_compile -/// # use agama_lib::settings::{Settings, SettingValue}; +/// # use agama_settings::settings::{Settings, SettingValue}; /// # use agama_derive::Settings; /// /// #[derive(Settings)] @@ -82,7 +82,7 @@ pub trait Settings { /// more types. /// /// ``` -/// # use agama_lib::settings::SettingValue; +/// # use agama_settings::settings::SettingValue; // /// let value = SettingValue("true".to_string()); /// let value: bool = value.try_into().expect("the conversion failed"); @@ -180,15 +180,3 @@ mod tests { assert_eq!(value, "some value"); } } - -#[derive(Error, Debug)] -pub enum SettingsError { - #[error("Unknown attribute '{0}'")] - UnknownAttribute(String), - #[error("Unknown collection '{0}'")] - UnknownCollection(String), - #[error("Invalid value '{0}'")] - InvalidValue(String), - #[error("Missing key '{0}'")] - MissingKey(String), -} From bf4310373b3b5e915388933eef4d4ef5a144f811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 18 Jul 2023 15:43:59 +0100 Subject: [PATCH 07/18] Avoid having to explictly 'use' the Settings trait --- rust/agama-derive/src/lib.rs | 2 +- rust/agama-lib/src/install_settings.rs | 1 - rust/agama-lib/src/network/settings.rs | 4 ++-- rust/agama-lib/src/software/settings.rs | 1 - rust/agama-lib/src/storage/settings.rs | 2 +- rust/agama-lib/src/users/settings.rs | 2 +- 6 files changed, 5 insertions(+), 7 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index 09512dac40..26fa8e751c 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -60,7 +60,7 @@ pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { let name = input.ident; let expanded = quote! { - impl Settings for #name { + impl agama_settings::settings::Settings for #name { #set_fn #add_fn #merge_fn diff --git a/rust/agama-lib/src/install_settings.rs b/rust/agama-lib/src/install_settings.rs index 27c641a96a..dbb842cd09 100644 --- a/rust/agama-lib/src/install_settings.rs +++ b/rust/agama-lib/src/install_settings.rs @@ -1,7 +1,6 @@ //! Configuration settings handling //! //! This module implements the mechanisms to load and store the installation settings. -use agama_settings::settings::Settings; use crate::{ network::NetworkSettings, software::SoftwareSettings, storage::StorageSettings, users::UserSettings, diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index adf4deecb7..e63eda9cc4 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -1,7 +1,7 @@ //! Representation of the network settings use super::types::DeviceType; -use agama_settings::{SettingsError, SettingObject, SettingValue, settings::Settings}; +use agama_settings::{SettingsError, SettingObject, SettingValue}; use agama_derive::Settings; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; @@ -78,7 +78,7 @@ impl TryFrom for NetworkConnection { #[cfg(test)] mod tests { use super::*; - use agama_settings::settings::{SettingObject, SettingValue}; + use agama_settings::{SettingObject, SettingValue, settings::Settings}; use std::collections::HashMap; #[test] diff --git a/rust/agama-lib/src/software/settings.rs b/rust/agama-lib/src/software/settings.rs index bb85c61136..d3851ef4d2 100644 --- a/rust/agama-lib/src/software/settings.rs +++ b/rust/agama-lib/src/software/settings.rs @@ -1,6 +1,5 @@ //! Representation of the software settings -use agama_settings::settings::Settings; use agama_derive::Settings; use serde::{Deserialize, Serialize}; diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs index 794c3f5641..44ea11d01a 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::{SettingsError, SettingObject, settings::Settings}; +use agama_settings::{SettingsError, SettingObject}; use agama_derive::Settings; use serde::{Deserialize, Serialize}; diff --git a/rust/agama-lib/src/users/settings.rs b/rust/agama-lib/src/users/settings.rs index 783f10f4fd..9cc3beeed2 100644 --- a/rust/agama-lib/src/users/settings.rs +++ b/rust/agama-lib/src/users/settings.rs @@ -1,4 +1,3 @@ -use agama_settings::settings::Settings; use agama_derive::Settings; use serde::{Deserialize, Serialize}; @@ -47,6 +46,7 @@ pub struct RootUserSettings { #[cfg(test)] mod tests { use super::*; + use agama_settings::settings::Settings; #[test] fn test_user_settings_merge() { From 164785e485e5dc0f0e6e459d40ce7d61bffa9ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 18 Jul 2023 16:12:18 +0100 Subject: [PATCH 08/18] Export Settings derive macro through agama_settings --- rust/Cargo.lock | 1 - rust/agama-lib/Cargo.toml | 3 +-- rust/agama-lib/src/install_settings.rs | 2 +- rust/agama-lib/src/network/settings.rs | 5 ++--- rust/agama-lib/src/software/settings.rs | 2 +- rust/agama-lib/src/storage/settings.rs | 3 +-- rust/agama-lib/src/users/settings.rs | 2 +- rust/agama-settings/src/lib.rs | 3 ++- rust/agama-settings/src/settings.rs | 3 +-- 9 files changed, 10 insertions(+), 14 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 9f25fad680..1260d60b6b 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -58,7 +58,6 @@ dependencies = [ name = "agama-lib" version = "1.0.0" dependencies = [ - "agama-derive", "agama-settings", "anyhow", "async-std", diff --git a/rust/agama-lib/Cargo.toml b/rust/agama-lib/Cargo.toml index cf63829e25..181a2cff50 100644 --- a/rust/agama-lib/Cargo.toml +++ b/rust/agama-lib/Cargo.toml @@ -6,7 +6,6 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -agama-derive = { path="../agama-derive" } agama-settings = { path="../agama-settings" } anyhow = "1.0" async-std = "1.12.0" @@ -19,4 +18,4 @@ serde = { version = "1.0.152", features = ["derive"] } serde_json = "1.0.94" tempfile = "3.4.0" thiserror = "1.0.39" -zbus = "3.7.0" \ No newline at end of file +zbus = "3.7.0" diff --git a/rust/agama-lib/src/install_settings.rs b/rust/agama-lib/src/install_settings.rs index dbb842cd09..727390144f 100644 --- a/rust/agama-lib/src/install_settings.rs +++ b/rust/agama-lib/src/install_settings.rs @@ -5,7 +5,7 @@ use crate::{ network::NetworkSettings, software::SoftwareSettings, storage::StorageSettings, users::UserSettings, }; -use agama_derive::Settings; +use agama_settings::Settings; use serde::{Deserialize, Serialize}; use std::default::Default; use std::str::FromStr; diff --git a/rust/agama-lib/src/network/settings.rs b/rust/agama-lib/src/network/settings.rs index e63eda9cc4..d9fdf3b18d 100644 --- a/rust/agama-lib/src/network/settings.rs +++ b/rust/agama-lib/src/network/settings.rs @@ -1,8 +1,7 @@ //! Representation of the network settings use super::types::DeviceType; -use agama_settings::{SettingsError, SettingObject, SettingValue}; -use agama_derive::Settings; +use agama_settings::{SettingObject, SettingValue, Settings, SettingsError}; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::default::Default; @@ -78,7 +77,7 @@ impl TryFrom for NetworkConnection { #[cfg(test)] mod tests { use super::*; - use agama_settings::{SettingObject, SettingValue, settings::Settings}; + use agama_settings::{settings::Settings, SettingObject, SettingValue}; use std::collections::HashMap; #[test] diff --git a/rust/agama-lib/src/software/settings.rs b/rust/agama-lib/src/software/settings.rs index d3851ef4d2..9458b964d3 100644 --- a/rust/agama-lib/src/software/settings.rs +++ b/rust/agama-lib/src/software/settings.rs @@ -1,6 +1,6 @@ //! Representation of the software settings -use agama_derive::Settings; +use agama_settings::Settings; use serde::{Deserialize, Serialize}; /// Software settings for installation diff --git a/rust/agama-lib/src/storage/settings.rs b/rust/agama-lib/src/storage/settings.rs index 44ea11d01a..ea64dce023 100644 --- a/rust/agama-lib/src/storage/settings.rs +++ b/rust/agama-lib/src/storage/settings.rs @@ -1,7 +1,6 @@ //! Representation of the storage settings -use agama_settings::{SettingsError, SettingObject}; -use agama_derive::Settings; +use agama_settings::{SettingObject, Settings, SettingsError}; use serde::{Deserialize, Serialize}; /// Storage settings for installation diff --git a/rust/agama-lib/src/users/settings.rs b/rust/agama-lib/src/users/settings.rs index 9cc3beeed2..2ec84e268a 100644 --- a/rust/agama-lib/src/users/settings.rs +++ b/rust/agama-lib/src/users/settings.rs @@ -1,4 +1,4 @@ -use agama_derive::Settings; +use agama_settings::Settings; use serde::{Deserialize, Serialize}; /// User settings diff --git a/rust/agama-settings/src/lib.rs b/rust/agama-settings/src/lib.rs index 172afabe3a..6a2f95d51e 100644 --- a/rust/agama-settings/src/lib.rs +++ b/rust/agama-settings/src/lib.rs @@ -2,4 +2,5 @@ pub mod error; pub mod settings; pub use self::error::SettingsError; -pub use self::settings::{SettingObject, SettingValue}; \ No newline at end of file +pub use self::settings::{SettingObject, SettingValue}; +pub use agama_derive::Settings; diff --git a/rust/agama-settings/src/settings.rs b/rust/agama-settings/src/settings.rs index 302b749651..8251e3e2c2 100644 --- a/rust/agama-settings/src/settings.rs +++ b/rust/agama-settings/src/settings.rs @@ -27,8 +27,7 @@ use std::fmt::Display; /// `UserSettings`. /// /// ```no_compile -/// # use agama_settings::settings::{Settings, SettingValue}; -/// # use agama_derive::Settings; +/// # use agama_settings::{Settings, settings::{Settings, SettingValue}}; /// /// #[derive(Settings)] /// struct UserSettings { From c42cb58ca3342268b8c4e2caa2987cce92ddc334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 18 Jul 2023 16:40:26 +0100 Subject: [PATCH 09/18] Add tests for the Settings derive macro --- rust/agama-settings/tests/settings.rs | 74 +++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 rust/agama-settings/tests/settings.rs diff --git a/rust/agama-settings/tests/settings.rs b/rust/agama-settings/tests/settings.rs new file mode 100644 index 0000000000..6fa8703ead --- /dev/null +++ b/rust/agama-settings/tests/settings.rs @@ -0,0 +1,74 @@ +use agama_settings::settings::Settings; +use agama_settings::{SettingObject, SettingValue, Settings, SettingsError}; +use std::collections::HashMap; + +/// Main settings +#[derive(Debug, Default, Settings)] +pub struct Main { + product: Option, + #[settings(collection)] + patterns: Vec, +} + +/// Software patterns +#[derive(Debug, Clone)] +pub struct Pattern { + id: String, +} + +/// TODO: deriving this trait could be easy. +impl TryFrom for Pattern { + type Error = SettingsError; + + fn try_from(value: SettingObject) -> Result { + match value.get("id") { + Some(id) => Ok(Pattern { + id: id.clone().try_into()?, + }), + _ => Err(SettingsError::MissingKey("id".to_string())), + } + } +} + +#[test] +fn test_set() { + let mut main = Main::default(); + main.set("product", SettingValue("Tumbleweed".to_string())) + .unwrap(); + + assert_eq!(main.product, Some("Tumbleweed".to_string())); + assert!(matches!( + main.set("missing", SettingValue("".to_string())), + Err(SettingsError::UnknownAttribute(_)) + )); +} + +#[test] +fn test_add() { + let mut main = Main::default(); + let pattern = HashMap::from([("id".to_string(), SettingValue("base".to_string()))]); + main.add("patterns", SettingObject(pattern)).unwrap(); + + let pattern = main.patterns.first().unwrap(); + assert_eq!(pattern.id, "base"); +} + +#[test] +fn test_merge() { + let mut main0 = Main { + product: Some("Tumbleweed".to_string()), + ..Default::default() + }; + + let patterns = vec![Pattern { + id: "enhanced".to_string(), + }]; + let main1 = Main { + product: Some("ALP".to_string()), + patterns, + }; + + main0.merge(&main1); + assert_eq!(main0.product, Some("ALP".to_string())); + assert_eq!(main0.patterns.len(), 1); +} From edef031996d63d648c374ba34d2e60aa88e29389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 17 Jul 2023 13:09:12 +0100 Subject: [PATCH 10/18] Update the changes file --- rust/package/agama-cli.changes | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/rust/package/agama-cli.changes b/rust/package/agama-cli.changes index 20902ea8d0..b7acc9f760 100644 --- a/rust/package/agama-cli.changes +++ b/rust/package/agama-cli.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Tue Jul 18 15:42:30 UTC 2023 - Imobach Gonzalez Sosa + +- Move the settings functionality to a separate package, + agama-settings (gh#openSUSE/agama#666). +- Make the "Settings" derive macro reusable from other crates. +- Extend the "Settings" derive macro to generate code for + InstallSettings and NetworkSettings. + ------------------------------------------------------------------- Tue Jul 18 13:32:04 UTC 2023 - Josef Reidinger From 9871226bed43f0c88a5259153dce733c41c2da25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 31 Jul 2023 07:55:36 +0100 Subject: [PATCH 11/18] Apply documentation suggestions from code review Co-authored-by: Martin Vidner --- rust/agama-derive/src/lib.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index 26fa8e751c..c323ce2e73 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -5,15 +5,18 @@ use syn::{parse_macro_input, DeriveInput, Fields}; #[derive(Debug, Clone, Copy, PartialEq)] enum SettingKind { + /// A single value; the default. Scalar, + /// An array of scalars, use `#[settings(collection)]` Collection, + /// The value is another FooSettings, use `#[settings(nested)]` Nested, } /// Represents a setting and its configuration #[derive(Debug, Clone)] struct SettingField { - /// Setting ident + /// Setting ident ("A word of Rust code, may be a keyword or variable name") pub ident: syn::Ident, /// Setting kind (scalar, collection, struct). pub kind: SettingKind, @@ -192,7 +195,10 @@ fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 { } } -// Extracts information about the settings fields +// Extracts information about the settings fields. +// +// syn::Field is "A field of a struct or enum variant.", +// has .ident .ty(pe) .mutability .vis(ibility)... fn parse_setting_fields(fields: Vec<&syn::Field>) -> SettingFieldsList { let mut settings = vec![]; for field in fields { From a0b7061ef415510d9435e38a00365c1a7b0e2d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 31 Jul 2023 07:56:33 +0100 Subject: [PATCH 12/18] Fix documentation referecne to the `Settings` trait Co-authored-by: Martin Vidner --- rust/agama-derive/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index c323ce2e73..c35926802a 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -42,7 +42,7 @@ impl SettingFieldsList { } /// Derive Settings, typically for a FooSettings struct. -/// (see agama_lib::settings::Settings but I cannot link to it without a circular dependency) +/// (see the trait agama_settings::settings::Settings but I cannot link to it without a circular dependency) #[proc_macro_derive(Settings, attributes(settings))] pub fn agama_attributes_derive(input: TokenStream) -> TokenStream { let input = parse_macro_input!(input as DeriveInput); From 02bb08e5c39491fc0fc33d0d849e80773c0c979a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 31 Jul 2023 08:20:30 +0100 Subject: [PATCH 13/18] Add the expected type to the InvalidValue error --- rust/agama-settings/src/error.rs | 4 ++-- rust/agama-settings/src/settings.rs | 13 ++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/rust/agama-settings/src/error.rs b/rust/agama-settings/src/error.rs index eb960e4e3b..27afd229a6 100644 --- a/rust/agama-settings/src/error.rs +++ b/rust/agama-settings/src/error.rs @@ -6,8 +6,8 @@ pub enum SettingsError { UnknownAttribute(String), #[error("Unknown collection '{0}'")] UnknownCollection(String), - #[error("Invalid value '{0}'")] - InvalidValue(String), + #[error("Invalid value '{0}', expected a {1}")] + InvalidValue(String, String), // TODO: add the value type name #[error("Missing key '{0}'")] MissingKey(String), } diff --git a/rust/agama-settings/src/settings.rs b/rust/agama-settings/src/settings.rs index 8251e3e2c2..fc00111443 100644 --- a/rust/agama-settings/src/settings.rs +++ b/rust/agama-settings/src/settings.rs @@ -128,7 +128,10 @@ impl TryFrom for bool { match value.0.to_lowercase().as_str() { "true" | "yes" | "t" => Ok(true), "false" | "no" | "f" => Ok(false), - _ => Err(SettingsError::InvalidValue(value.to_string())), + _ => Err(SettingsError::InvalidValue( + value.to_string(), + "boolean".to_string(), + )), } } } @@ -170,6 +173,14 @@ mod tests { let value = SettingValue("false".to_string()); let value: bool = value.try_into().unwrap(); assert!(!value); + + let value = SettingValue("fasle".to_string()); + let value: Result = value.try_into(); + let error = value.unwrap_err(); + assert_eq!( + error.to_string(), + "Invalid value 'fasle', expected a boolean" + ); } #[test] From 8dbdbbc47cc9606e7f7002f8aa9370908d80db71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 31 Jul 2023 09:48:00 +0100 Subject: [PATCH 14/18] Fix combining nested and collection settings --- rust/agama-derive/src/lib.rs | 1 + rust/agama-settings/tests/settings.rs | 39 ++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index c35926802a..04cbf5f1c5 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -182,6 +182,7 @@ fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 { })* _ => return Err(agama_settings::SettingsError::UnknownAttribute(attr.to_string())) } + return Ok(()) } } } diff --git a/rust/agama-settings/tests/settings.rs b/rust/agama-settings/tests/settings.rs index 6fa8703ead..974d36f5b5 100644 --- a/rust/agama-settings/tests/settings.rs +++ b/rust/agama-settings/tests/settings.rs @@ -8,6 +8,8 @@ pub struct Main { product: Option, #[settings(collection)] patterns: Vec, + #[settings(nested)] + network: Option, } /// Software patterns @@ -16,6 +18,12 @@ pub struct Pattern { id: String, } +#[derive(Default, Debug, Settings)] +pub struct Network { + enabled: Option, + ssid: Option, +} + /// TODO: deriving this trait could be easy. impl TryFrom for Pattern { type Error = SettingsError; @@ -37,10 +45,32 @@ fn test_set() { .unwrap(); assert_eq!(main.product, Some("Tumbleweed".to_string())); - assert!(matches!( - main.set("missing", SettingValue("".to_string())), - Err(SettingsError::UnknownAttribute(_)) - )); + main.set("network.enabled", SettingValue("true".to_string())) + .unwrap(); + let network = main.network.unwrap(); + assert_eq!(network.enabled, Some(true)); +} + +#[test] +fn test_set_unknown_attribute() { + let mut main = Main::default(); + let error = main + .set("missing", SettingValue("".to_string())) + .unwrap_err(); + assert_eq!(error.to_string(), "Unknown attribute 'missing'"); +} + +#[test] +fn test_invalid_set() { + let mut main = Main::default(); + + let error = main + .set("network.enabled", SettingValue("fasle".to_string())) + .unwrap_err(); + assert_eq!( + error.to_string(), + "Invalid value 'fasle', expected a boolean" + ); } #[test] @@ -66,6 +96,7 @@ fn test_merge() { let main1 = Main { product: Some("ALP".to_string()), patterns, + ..Default::default() }; main0.merge(&main1); From 3becbc220299f87e60f8d542e21393550600b7fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 1 Aug 2023 12:48:46 +0100 Subject: [PATCH 15/18] Improve agama-derive documentation --- rust/agama-derive/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/agama-derive/src/lib.rs b/rust/agama-derive/src/lib.rs index 04cbf5f1c5..2f3bed78da 100644 --- a/rust/agama-derive/src/lib.rs +++ b/rust/agama-derive/src/lib.rs @@ -7,16 +7,16 @@ use syn::{parse_macro_input, DeriveInput, Fields}; enum SettingKind { /// A single value; the default. Scalar, - /// An array of scalars, use `#[settings(collection)]` + /// An array of scalars, use `#[settings(collection)]`. Collection, - /// The value is another FooSettings, use `#[settings(nested)]` + /// The value is another FooSettings, use `#[settings(nested)]`. Nested, } /// Represents a setting and its configuration #[derive(Debug, Clone)] struct SettingField { - /// Setting ident ("A word of Rust code, may be a keyword or variable name") + /// Setting ident ("A word of Rust code, may be a keyword or variable name"). pub ident: syn::Ident, /// Setting kind (scalar, collection, struct). pub kind: SettingKind, @@ -198,7 +198,7 @@ fn expand_add_fn(settings: &SettingFieldsList) -> TokenStream2 { // Extracts information about the settings fields. // -// syn::Field is "A field of a struct or enum variant.", +// syn::Field is "A field of a struct or enum variant.", // has .ident .ty(pe) .mutability .vis(ibility)... fn parse_setting_fields(fields: Vec<&syn::Field>) -> SettingFieldsList { let mut settings = vec![]; 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 16/18] 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" ); } From 170690ed421c3daf046a52ad21eefb5d6a4ac64d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 2 Aug 2023 08:22:04 +0100 Subject: [PATCH 17/18] Fix a broken link in the agama-lib docs --- rust/agama-lib/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/agama-lib/src/lib.rs b/rust/agama-lib/src/lib.rs index 5436218f9b..c0ba6866e3 100644 --- a/rust/agama-lib/src/lib.rs +++ b/rust/agama-lib/src/lib.rs @@ -15,8 +15,8 @@ //! Each of those modules contains, at least: //! //! * A settings model: it is a representation of the installation settings for the given topic. It -//! is expected to implement the [serde::Serialize], [serde::Deserialize] and [settings::Settings] -//! traits. +//! is expected to implement the [serde::Serialize], [serde::Deserialize] and +//! [agama_settings::settings::Settings] traits. //! * A store: it is the responsible for reading/writing the settings to the service. Usually, it //! relies on a D-Bus client for communicating with the service, although it could implement that //! logic itself. Note: we are considering defining a trait for stores too. From 0323f0fedc340bf996f7f5ba5826d49c63d2f0c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 2 Aug 2023 09:01:02 +0100 Subject: [PATCH 18/18] Update the changes file --- rust/package/agama-cli.changes | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rust/package/agama-cli.changes b/rust/package/agama-cli.changes index fa8fb57f97..bb901a3a72 100644 --- a/rust/package/agama-cli.changes +++ b/rust/package/agama-cli.changes @@ -6,6 +6,8 @@ Mon Jul 31 06:58:15 UTC 2023 - Imobach Gonzalez Sosa - Make the "Settings" derive macro reusable from other crates. - Extend the "Settings" derive macro to generate code for InstallSettings and NetworkSettings. +- Improve error reporting when working with the "config" + subcommand. ------------------------------------------------------------------- Wed Jul 26 11:08:09 UTC 2023 - Josef Reidinger