From 7c787edef028ed0ae6bed40af4ca9423d664f10c Mon Sep 17 00:00:00 2001 From: Lorenzo Bertin Salvador Date: Sun, 23 Mar 2025 10:18:12 -0300 Subject: [PATCH 1/5] refactor: improve code quality in config.rs module This commit improves code quality in config.rs module by 1. implementing std::Default instead of using a loose fn default() 2. creating a get_config_path function, which centralizes config path build and therefore helps remove duplicate code. 3. changing build config logic to save Config struct back to config file right after loading it. The improvement number 3 was done in case loaded Config struct is somehow different from the saved one. This will be crucial for solving issue #118. Signed-off-by: Lorenzo Bertin Salvador --- src/app/config.rs | 64 +++++++++++++++++++---------------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/src/app/config.rs b/src/app/config.rs index 484ee6ef..a1134cb4 100644 --- a/src/app/config.rs +++ b/src/app/config.rs @@ -8,6 +8,8 @@ use std::{ path::Path, }; +pub const DEFAULT_CONFIG_PATH_SUFFIX: &str = ".config/patch-hub/config.json"; + use super::{cover_renderer::CoverRenderer, patch_renderer::PatchRenderer}; #[cfg(test)] @@ -52,7 +54,7 @@ pub struct KernelTree { branch: String, } -impl Config { +impl Default for Config { fn default() -> Self { let home = env::var("HOME").unwrap_or_else(|_| { eprintln!("$HOME environment variable not set, using current directory"); @@ -80,33 +82,19 @@ impl Config { git_am_branch_prefix: String::from("patchset-"), } } +} +impl Config { /// Loads the configuration for patch-hub from the config file. /// - /// Returns `None` if the config file is not found or if it's not a valid JSON. - fn load_file() -> Option { - if let Ok(config_path) = env::var("PATCH_HUB_CONFIG_PATH") { - if Path::new(&config_path).is_file() { - match fs::read_to_string(&config_path) { - Ok(file_contents) => match serde_json::from_str(&file_contents) { - Ok(config) => return Some(config), - Err(e) => eprintln!("Failed to parse config file {}: {}", config_path, e), - }, - Err(e) => { - eprintln!("Failed to read config file {}: {}", config_path, e) - } - } - } - } + /// Returns the default config if the config file is not found or if it's not a valid JSON. + fn load_file() -> Config { + let config_path = Config::get_config_path(); - let config_path = format!( - "{}/.config/patch-hub/config.json", - env::var("HOME").unwrap() - ); if Path::new(&config_path).is_file() { match fs::read_to_string(&config_path) { Ok(file_contents) => match serde_json::from_str(&file_contents) { - Ok(config) => return Some(config), + Ok(config) => return config, Err(e) => eprintln!("Failed to parse config file {}: {}", config_path, e), }, Err(e) => { @@ -115,7 +103,7 @@ impl Config { } } - None + Config::default() } fn override_with_env_vars(&mut self) { @@ -141,15 +129,10 @@ impl Config { } pub fn build() -> Self { - let mut config = Self::load_file().unwrap_or_else(|| { - eprintln!("No valid config file found, using default configuration"); - let config = Self::default(); - config.save_patch_hub_config().unwrap_or_else(|e| { - eprintln!("Failed to save default config: {}", e); - }); - config + let mut config = Self::load_file(); + config.save_patch_hub_config().unwrap_or_else(|e| { + eprintln!("Failed to save default config: {}", e); }); - config.override_with_env_vars(); config @@ -214,14 +197,7 @@ impl Config { } pub fn save_patch_hub_config(&self) -> io::Result<()> { - let config_path = if let Ok(path) = env::var("PATCH_HUB_CONFIG_PATH") { - path - } else { - format!( - "{}/.config/patch-hub/config.json", - env::var("HOME").unwrap() - ) - }; + let config_path = Config::get_config_path(); let config_path = Path::new(&config_path); // We need to assure that the parent dir of `config_path` exists @@ -238,6 +214,18 @@ impl Config { Ok(()) } + /// Returns the current Config path + /// + /// It tries to get direct config path from env var PATCH_HUB_CONFIG_PATH, + /// otherwise it uses $HOME + suffix + fn get_config_path() -> String { + env::var("PATCH_HUB_CONFIG_PATH").unwrap_or(format!( + "{}/{}", + env::var("HOME").unwrap(), + DEFAULT_CONFIG_PATH_SUFFIX + )) + } + /// Creates the needed directories if they don't exist. /// The directories are defined during the Config build. /// From 5cb869b56f2daf53bfbfa09adfeff048080eda6a Mon Sep 17 00:00:00 2001 From: Lorenzo Bertin Salvador Date: Sun, 23 Mar 2025 10:50:38 -0300 Subject: [PATCH 2/5] feat: prevent existing config to be overwritten when new attribute is created This solves #118 by creating individual default functions for each Config attribute, so if I new attribute is created, an existing config file will not fail to serialize, it will just create the new attribute with its default value. Signed-off-by: Lorenzo Bertin Salvador --- src/app/config.rs | 82 ++++++++++++++++++++++++- src/app/config/tests.rs | 18 ++++++ src/test_samples/app/config/config.json | 4 +- 3 files changed, 101 insertions(+), 3 deletions(-) diff --git a/src/app/config.rs b/src/app/config.rs index a1134cb4..eac9bde1 100644 --- a/src/app/config.rs +++ b/src/app/config.rs @@ -18,31 +18,47 @@ mod tests; #[derive(Serialize, Deserialize, Getters)] pub struct Config { #[getter(skip)] + #[serde(default = "default_page_size")] page_size: usize, + #[serde(default = "default_patchsets_cache_dir")] patchsets_cache_dir: String, + #[serde(default = "default_bookmarked_patchsets_path")] bookmarked_patchsets_path: String, + #[serde(default = "default_mailing_lists_path")] mailing_lists_path: String, + #[serde(default = "default_reviewed_patchsets_path")] reviewed_patchsets_path: String, /// Logs directory + #[serde(default = "default_logs_path")] logs_path: String, + #[serde(default = "default_git_send_email_options")] git_send_email_options: String, /// Base directory for all patch-hub cache + #[serde(default = "default_cache_dir")] cache_dir: String, /// Base directory for all patch-hub cache + #[serde(default = "default_data_dir")] data_dir: String, /// Renderer to use for patch previews + #[serde(default = "default_patch_renderer")] patch_renderer: PatchRenderer, /// Renderer to use for patchset covers + #[serde(default = "default_cover_renderer")] cover_renderer: CoverRenderer, /// Maximum age of a log file in days + #[serde(default = "default_max_log_age")] max_log_age: usize, #[getter(skip)] /// Map of tracked kernel trees + #[serde(default = "default_kernel_trees")] kernel_trees: HashMap, /// Target kernel tree to run actions + #[serde(default = "default_target_kernel_tree")] target_kernel_tree: Option, - /// Flags to be use with `git am` command when applying patches + /// Flags to be use with git am command when applying patches + #[serde(default = "default_git_am_options")] git_am_options: String, + #[serde(default = "default_git_am_branch_help")] git_am_branch_prefix: String, } @@ -245,3 +261,67 @@ impl Config { } } } + +fn default_page_size() -> usize { + Config::default().page_size +} + +fn default_patchsets_cache_dir() -> String { + Config::default().patchsets_cache_dir +} + +fn default_bookmarked_patchsets_path() -> String { + Config::default().bookmarked_patchsets_path +} + +fn default_mailing_lists_path() -> String { + Config::default().mailing_lists_path +} + +fn default_reviewed_patchsets_path() -> String { + Config::default().reviewed_patchsets_path +} + +fn default_logs_path() -> String { + Config::default().logs_path +} + +fn default_git_send_email_options() -> String { + Config::default().git_send_email_options +} + +fn default_cache_dir() -> String { + Config::default().cache_dir +} + +fn default_data_dir() -> String { + Config::default().data_dir +} + +fn default_patch_renderer() -> PatchRenderer { + Config::default().patch_renderer +} + +fn default_cover_renderer() -> CoverRenderer { + Config::default().cover_renderer +} + +fn default_max_log_age() -> usize { + Config::default().max_log_age +} + +fn default_kernel_trees() -> HashMap { + Config::default().kernel_trees +} + +fn default_target_kernel_tree() -> Option { + Config::default().target_kernel_tree +} + +fn default_git_am_options() -> String { + Config::default().git_am_options +} + +fn default_git_am_branch_help() -> String { + Config::default().git_am_branch_prefix +} diff --git a/src/app/config/tests.rs b/src/app/config/tests.rs index d5cd6ee5..3c1e79fb 100644 --- a/src/app/config/tests.rs +++ b/src/app/config/tests.rs @@ -1,3 +1,5 @@ +use serde_json::json; + use super::*; use std::sync::Mutex; @@ -148,3 +150,19 @@ fn test_config_precedence() { env::remove_var("PATCH_HUB_CONFIG_PATH"); env::remove_var("PATCH_HUB_PAGE_SIZE"); } + +#[test] +fn test_deserialize_config_with_missing_field() { + // Example JSON string that doesn't contain `page_size` but has `max_log_age` set to 500. + let json_data = json!({ + "max_log_age": 500 + }); + + let config: Config = serde_json::from_value(json_data).unwrap(); + + // Assert that `page_size` is set to the default value (25) + assert_eq!(config.page_size, 30); + + // Assert that `max_log_age` is set to the custom value + assert_eq!(config.max_log_age, 500); +} diff --git a/src/test_samples/app/config/config.json b/src/test_samples/app/config/config.json index 05e2b5fe..36b16842 100644 --- a/src/test_samples/app/config/config.json +++ b/src/test_samples/app/config/config.json @@ -4,7 +4,7 @@ "bookmarked_patchsets_path": "/bookmarked/patchsets/path", "mailing_lists_path": "/mailing/lists/path", "reviewed_patchsets_path": "/reviewed/patchsets/path", - "logs_path":"/logs/path", + "logs_path": "/logs/path", "git_send_email_options": "--long-option value -s -h -o -r -t", "cache_dir": "/cache_dir", "data_dir": "/data_dir", @@ -24,4 +24,4 @@ "target_kernel_tree": "linux", "git_am_options": "--foo-bar foobar -s -n -o -r -l -a -x", "git_am_branch_prefix": "really-creative-prefix-" -} +} \ No newline at end of file From 157ec1c0fa91ec9d15e6648870180bbdb4b50780 Mon Sep 17 00:00:00 2001 From: Lorenzo Bertin Salvador Date: Sun, 23 Mar 2025 11:00:01 -0300 Subject: [PATCH 3/5] refactor: use lazy_static to store only one Config::default() object in memory This is the first optimization for Config individual attributes serialization. With this, we don't need to create a new Config object each time we access the default attribute functions (normally in deserialization). Signed-off-by: Lorenzo Bertin Salvador --- Cargo.lock | 1 + Cargo.toml | 1 + src/app/config.rs | 40 +++++++++++++++++++++++----------------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7f78fc30..cbb3d5b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -764,6 +764,7 @@ dependencies = [ "clap", "color-eyre", "derive-getters", + "lazy_static", "mockall", "ratatui", "regex", diff --git a/Cargo.toml b/Cargo.toml index 9711f937..eccc40b5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ description = "patch-hub is a TUI that streamlines the interaction of Linux deve color-eyre = "0.6.3" mockall = "0.13.0" derive-getters = { version = "0.5.0", features = ["auto_copy_getters"] } +lazy_static = "1.5.0" ratatui = "0.28.1" regex = "1.10.5" serde = { version = "1.0.203", features = ["derive"] } diff --git a/src/app/config.rs b/src/app/config.rs index eac9bde1..a814e833 100644 --- a/src/app/config.rs +++ b/src/app/config.rs @@ -1,4 +1,5 @@ use derive_getters::Getters; +use lazy_static::lazy_static; use serde::{Deserialize, Serialize}; use std::{ collections::{HashMap, HashSet}, @@ -8,6 +9,11 @@ use std::{ path::Path, }; +// store only one Config::default object in memory +lazy_static! { + static ref DEFAULT_CONFIG: Config = Config::default(); +} + pub const DEFAULT_CONFIG_PATH_SUFFIX: &str = ".config/patch-hub/config.json"; use super::{cover_renderer::CoverRenderer, patch_renderer::PatchRenderer}; @@ -62,7 +68,7 @@ pub struct Config { git_am_branch_prefix: String, } -#[derive(Debug, Serialize, Deserialize, Getters, Eq, PartialEq)] +#[derive(Debug, Serialize, Deserialize, Getters, Eq, PartialEq, Clone)] pub struct KernelTree { /// Path to kernel tree in the filesystem path: String, @@ -263,65 +269,65 @@ impl Config { } fn default_page_size() -> usize { - Config::default().page_size + DEFAULT_CONFIG.page_size } fn default_patchsets_cache_dir() -> String { - Config::default().patchsets_cache_dir + DEFAULT_CONFIG.patchsets_cache_dir.clone() } fn default_bookmarked_patchsets_path() -> String { - Config::default().bookmarked_patchsets_path + DEFAULT_CONFIG.bookmarked_patchsets_path.clone() } fn default_mailing_lists_path() -> String { - Config::default().mailing_lists_path + DEFAULT_CONFIG.mailing_lists_path.clone() } fn default_reviewed_patchsets_path() -> String { - Config::default().reviewed_patchsets_path + DEFAULT_CONFIG.reviewed_patchsets_path.clone() } fn default_logs_path() -> String { - Config::default().logs_path + DEFAULT_CONFIG.logs_path.clone() } fn default_git_send_email_options() -> String { - Config::default().git_send_email_options + DEFAULT_CONFIG.git_send_email_options.clone() } fn default_cache_dir() -> String { - Config::default().cache_dir + DEFAULT_CONFIG.cache_dir.clone() } fn default_data_dir() -> String { - Config::default().data_dir + DEFAULT_CONFIG.data_dir.clone() } fn default_patch_renderer() -> PatchRenderer { - Config::default().patch_renderer + DEFAULT_CONFIG.patch_renderer } fn default_cover_renderer() -> CoverRenderer { - Config::default().cover_renderer + DEFAULT_CONFIG.cover_renderer } fn default_max_log_age() -> usize { - Config::default().max_log_age + DEFAULT_CONFIG.max_log_age } fn default_kernel_trees() -> HashMap { - Config::default().kernel_trees + DEFAULT_CONFIG.kernel_trees.clone() } fn default_target_kernel_tree() -> Option { - Config::default().target_kernel_tree + DEFAULT_CONFIG.target_kernel_tree.clone() } fn default_git_am_options() -> String { - Config::default().git_am_options + DEFAULT_CONFIG.git_am_options.clone() } fn default_git_am_branch_help() -> String { - Config::default().git_am_branch_prefix + DEFAULT_CONFIG.git_am_branch_prefix.clone() } From 94da7f404ce24c08ccbd04cb06fa3120c3814c4c Mon Sep 17 00:00:00 2001 From: Lorenzo Bertin Salvador Date: Sun, 23 Mar 2025 15:45:34 -0300 Subject: [PATCH 4/5] feat: create proc_macro to simplify Config attributes deserialization This commit adds a procedural macro to patch-hub! The intention is to simplify the Config struct by avoiding to add a #[serde(default = "")] for each of its attributes and implement the default function. The proc_macro supports all kinds of structs and should preserve all other struct attributes, derivations and general behavior. This completely solves #118. Signed-off-by: Lorenzo Bertin Salvador --- Cargo.lock | 22 ++++++-- Cargo.toml | 6 +++ proc_macros/Cargo.toml | 17 ++++++ proc_macros/src/lib.rs | 117 +++++++++++++++++++++++++++++++++++++++++ src/app/config.rs | 90 ++----------------------------- 5 files changed, 161 insertions(+), 91 deletions(-) create mode 100644 proc_macros/Cargo.toml create mode 100644 proc_macros/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index cbb3d5b8..31c17257 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -766,6 +766,7 @@ dependencies = [ "derive-getters", "lazy_static", "mockall", + "proc_macros", "ratatui", "regex", "serde", @@ -816,13 +817,26 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.89" +version = "1.0.94" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f139b0662de085916d1fb67d2b4169d1addddda1919e696f3252b740b629986e" +checksum = "a31971752e70b8b2686d7e46ec17fb38dad4051d94024c88df49b667caea9c84" dependencies = [ "unicode-ident", ] +[[package]] +name = "proc_macros" +version = "0.1.0" +dependencies = [ + "derive-getters", + "lazy_static", + "proc-macro2", + "quote", + "serde", + "serde_json", + "syn", +] + [[package]] name = "quote" version = "1.0.37" @@ -1133,9 +1147,9 @@ checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" [[package]] name = "syn" -version = "2.0.85" +version = "2.0.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5023162dfcd14ef8f32034d8bcd4cc5ddc61ef7a247c024a33e24e1f24d21b56" +checksum = "b09a44accad81e1ba1cd74a32461ba89dee89095ba17b32f5d03683b1b1fc2a0" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index eccc40b5..9fd5ad5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ color-eyre = "0.6.3" mockall = "0.13.0" derive-getters = { version = "0.5.0", features = ["auto_copy_getters"] } lazy_static = "1.5.0" +proc_macros = { path = "./proc_macros" } ratatui = "0.28.1" regex = "1.10.5" serde = { version = "1.0.203", features = ["derive"] } @@ -48,3 +49,8 @@ unconditional_recursion = "deny" [lints.clippy] too-many-arguments = "allow" map_unwrap_or = "deny" + +[workspace] +members = [ + "proc_macros", +] diff --git a/proc_macros/Cargo.toml b/proc_macros/Cargo.toml new file mode 100644 index 00000000..865076ae --- /dev/null +++ b/proc_macros/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "proc_macros" +version = "0.1.0" +edition = "2021" + +[dependencies] +derive-getters = { version = "0.5.0", features = ["auto_copy_getters"] } +lazy_static = "1.5.0" +serde = { version = "1.0.203", features = ["derive"] } +serde_json = "1.0.120" +syn = { version = "2.0.100", features = ["full"] } +quote = "1.0.4" +proc-macro2 = "1.0.94" + +[lib] +proc-macro = true +doctest = false # otherwise tests will fail diff --git a/proc_macros/src/lib.rs b/proc_macros/src/lib.rs new file mode 100644 index 00000000..61bc63a2 --- /dev/null +++ b/proc_macros/src/lib.rs @@ -0,0 +1,117 @@ +extern crate proc_macro; +use proc_macro::TokenStream; +use quote::{format_ident, quote}; +use syn::{parse_macro_input, Data, DeriveInput}; + +/// This procedural macro create default deserealization functions for each +/// structure attribute based on std::Default impl. +/// +/// It applies default values only for fields missing from the deserialized input. +/// +/// # Example +/// +/// ```rust +/// use serde::Deserialize; +/// use your_proc_macro_crate::serde_individual_default; +/// +/// #[derive(Deserialize, Getters)] +/// #[serde_individual_default] +/// struct Example { +/// #[getter(skip)] +/// test_1: i64, +/// test_2: i64, +/// test_3: String, +/// } +/// impl Default for Example { +/// fn default() -> Self { +/// Example { +/// test_1: 3942, +/// test_2: 42390, +/// test_3: "a".to_string(), +/// } +/// } +/// } +/// +/// let json_data_1 = serde_json::json!({ +/// "test_1": 500, +/// "test_2": 100 +/// }); +/// let example_struct_1: Example = serde_json::from_value(json_data_1).unwrap(); +/// assert_eq!(example_struct_1.test_1, 500); +/// assert_eq!(example_struct_1.test_2, 100); +/// assert_eq!(example_struct_1.test_3, "a".to_string()); +/// ``` +#[proc_macro_attribute] +pub fn serde_individual_default(_attr: TokenStream, input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as DeriveInput); + let struct_name = &input.ident; + let struct_generics = &input.generics; + let struct_fields = match &input.data { + Data::Struct(s) => &s.fields, + _ => panic!("SerdeIndividualDefault can only be used with structs"), + }; + let struct_attrs = &input.attrs; + let struct_visibility = &input.vis; + + let struct_name_str = struct_name.to_string(); + let (struct_impl_generics, struct_ty_generics, struct_where_clause) = + struct_generics.split_for_impl(); + + // store only one struct::default object in memory + let default_config_struct_name = + format_ident!("DEFAULT_{}_STATIC", struct_name_str.to_ascii_uppercase()); + let struct_default_lazy_construction_definition = { + quote! { + lazy_static::lazy_static! { + static ref #default_config_struct_name: #struct_name = #struct_name::default(); + } + } + }; + + // build struct attributes with #[serde(default = "")] and build the default function itself + let (all_field_attrs, default_deserialize_function_definitions) = struct_fields.iter().fold( + (vec![], vec![]), + |(mut all_field_attrs, mut default_deserialize_function_definitions), field| { + let field_name = &field.ident; + let field_type = &field.ty; + let field_vis = &field.vis; + let field_attrs = &field.attrs; + let field_name_str = field_name.as_ref().unwrap().to_string(); + + // default function name will be named default_{struct_name}_{field_name} + let default_deserialize_function_name = + format_ident!("default_{}_{}", struct_name_str, field_name_str); + + let default_deserialize_function_name_str = + default_deserialize_function_name.to_string(); + + all_field_attrs.push(quote! { + #(#field_attrs)* + #[serde(default = #default_deserialize_function_name_str)] + #field_vis #field_name: #field_type, + }); + default_deserialize_function_definitions.push(quote! { + fn #default_deserialize_function_name() -> #field_type { + #default_config_struct_name.#field_name.clone() + } + }); + + (all_field_attrs, default_deserialize_function_definitions) + }, + ); + + // build final struct. + //We have to explicitly derive Deserialize here so the serde attribute works + let expanded_token_stream = quote! { + #[derive(serde::Deserialize)] + #(#struct_attrs)* + #struct_visibility struct #struct_name #struct_impl_generics { + #(#all_field_attrs)* + } #struct_ty_generics #struct_where_clause + + #struct_default_lazy_construction_definition + + #(#default_deserialize_function_definitions)* + }; + TokenStream::from(expanded_token_stream) +} diff --git a/src/app/config.rs b/src/app/config.rs index a814e833..60579f66 100644 --- a/src/app/config.rs +++ b/src/app/config.rs @@ -1,5 +1,5 @@ use derive_getters::Getters; -use lazy_static::lazy_static; +use proc_macros::serde_individual_default; use serde::{Deserialize, Serialize}; use std::{ collections::{HashMap, HashSet}, @@ -9,11 +9,6 @@ use std::{ path::Path, }; -// store only one Config::default object in memory -lazy_static! { - static ref DEFAULT_CONFIG: Config = Config::default(); -} - pub const DEFAULT_CONFIG_PATH_SUFFIX: &str = ".config/patch-hub/config.json"; use super::{cover_renderer::CoverRenderer, patch_renderer::PatchRenderer}; @@ -21,50 +16,35 @@ use super::{cover_renderer::CoverRenderer, patch_renderer::PatchRenderer}; #[cfg(test)] mod tests; -#[derive(Serialize, Deserialize, Getters)] +#[derive(Serialize, Getters)] +#[serde_individual_default] pub struct Config { #[getter(skip)] - #[serde(default = "default_page_size")] page_size: usize, - #[serde(default = "default_patchsets_cache_dir")] patchsets_cache_dir: String, - #[serde(default = "default_bookmarked_patchsets_path")] bookmarked_patchsets_path: String, - #[serde(default = "default_mailing_lists_path")] mailing_lists_path: String, - #[serde(default = "default_reviewed_patchsets_path")] reviewed_patchsets_path: String, /// Logs directory - #[serde(default = "default_logs_path")] logs_path: String, - #[serde(default = "default_git_send_email_options")] git_send_email_options: String, /// Base directory for all patch-hub cache - #[serde(default = "default_cache_dir")] cache_dir: String, /// Base directory for all patch-hub cache - #[serde(default = "default_data_dir")] data_dir: String, /// Renderer to use for patch previews - #[serde(default = "default_patch_renderer")] patch_renderer: PatchRenderer, /// Renderer to use for patchset covers - #[serde(default = "default_cover_renderer")] cover_renderer: CoverRenderer, /// Maximum age of a log file in days - #[serde(default = "default_max_log_age")] max_log_age: usize, #[getter(skip)] /// Map of tracked kernel trees - #[serde(default = "default_kernel_trees")] kernel_trees: HashMap, /// Target kernel tree to run actions - #[serde(default = "default_target_kernel_tree")] target_kernel_tree: Option, /// Flags to be use with git am command when applying patches - #[serde(default = "default_git_am_options")] git_am_options: String, - #[serde(default = "default_git_am_branch_help")] git_am_branch_prefix: String, } @@ -267,67 +247,3 @@ impl Config { } } } - -fn default_page_size() -> usize { - DEFAULT_CONFIG.page_size -} - -fn default_patchsets_cache_dir() -> String { - DEFAULT_CONFIG.patchsets_cache_dir.clone() -} - -fn default_bookmarked_patchsets_path() -> String { - DEFAULT_CONFIG.bookmarked_patchsets_path.clone() -} - -fn default_mailing_lists_path() -> String { - DEFAULT_CONFIG.mailing_lists_path.clone() -} - -fn default_reviewed_patchsets_path() -> String { - DEFAULT_CONFIG.reviewed_patchsets_path.clone() -} - -fn default_logs_path() -> String { - DEFAULT_CONFIG.logs_path.clone() -} - -fn default_git_send_email_options() -> String { - DEFAULT_CONFIG.git_send_email_options.clone() -} - -fn default_cache_dir() -> String { - DEFAULT_CONFIG.cache_dir.clone() -} - -fn default_data_dir() -> String { - DEFAULT_CONFIG.data_dir.clone() -} - -fn default_patch_renderer() -> PatchRenderer { - DEFAULT_CONFIG.patch_renderer -} - -fn default_cover_renderer() -> CoverRenderer { - DEFAULT_CONFIG.cover_renderer -} - -fn default_max_log_age() -> usize { - DEFAULT_CONFIG.max_log_age -} - -fn default_kernel_trees() -> HashMap { - DEFAULT_CONFIG.kernel_trees.clone() -} - -fn default_target_kernel_tree() -> Option { - DEFAULT_CONFIG.target_kernel_tree.clone() -} - -fn default_git_am_options() -> String { - DEFAULT_CONFIG.git_am_options.clone() -} - -fn default_git_am_branch_help() -> String { - DEFAULT_CONFIG.git_am_branch_prefix.clone() -} From a0ed8f57595d6481e11a77566265eaa56093f59d Mon Sep 17 00:00:00 2001 From: Lorenzo Bertin Salvador Date: Sun, 23 Mar 2025 15:50:18 -0300 Subject: [PATCH 5/5] feat: add unit tests for serde_individual_default proc_macro This commit adds unit tests for the serde_individual_default procedural macro. As a consequence, it is necessary to add the flag --all on cargo test so the new proc_macro tests are executed. Signed-off-by: Lorenzo Bertin Salvador --- .github/workflows/build_and_unit_test.yml | 2 +- proc_macros/tests/serde_individual_default.rs | 132 ++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 proc_macros/tests/serde_individual_default.rs diff --git a/.github/workflows/build_and_unit_test.yml b/.github/workflows/build_and_unit_test.yml index 115bc344..716ffcd4 100644 --- a/.github/workflows/build_and_unit_test.yml +++ b/.github/workflows/build_and_unit_test.yml @@ -44,7 +44,7 @@ jobs: - name: Run unit test suites shell: bash run: | - cargo test --all-targets --verbose && exit 0 + cargo test --all --all-targets --verbose && exit 0 printf '\e[1;33m\t==========================================\n\e[0m' printf '\e[1;33m\tUNIT TEST SUITE FAILED\n\e[0m' printf '\e[1;33m\tPLEASE, SOLVE THEM LOCALLY W/ `cargo test`\e[0m\n' diff --git a/proc_macros/tests/serde_individual_default.rs b/proc_macros/tests/serde_individual_default.rs new file mode 100644 index 00000000..40e6bbd9 --- /dev/null +++ b/proc_macros/tests/serde_individual_default.rs @@ -0,0 +1,132 @@ +use proc_macros::serde_individual_default; + +use derive_getters::Getters; +use serde::Serialize; + +#[derive(Serialize, Getters)] +#[serde_individual_default] +struct Example { + #[getter(skip)] + test_1: i64, + test_2: i64, + test_3: String, +} + +impl Default for Example { + fn default() -> Self { + Example { + test_1: 3942, + test_2: 42390, + test_3: "a".to_string(), + } + } +} + +#[serde_individual_default] +struct ExampleWithoutSerialize { + test_1: i64, + test_2: i64, +} + +impl Default for ExampleWithoutSerialize { + fn default() -> Self { + ExampleWithoutSerialize { + test_1: 765, + test_2: 126, + } + } +} + +#[serde_individual_default] +pub struct ExamplePublic { + test_1: i64, + test_2: i64, +} + +impl Default for ExamplePublic { + fn default() -> Self { + ExamplePublic { + test_1: 598, + test_2: 403, + } + } +} + +#[test] +fn should_have_default_serialization() { + // Case 1: test_3 missing + + // Example JSON string that doesn't contain `test_3` but has customized `test_1` and `test_2` + let json_data_1 = serde_json::json!({ + "test_1": 500, + "test_2": 100 + }); + + let example_struct_1: Example = serde_json::from_value(json_data_1).unwrap(); + + // Assert that`test_1` and `test_2` are set to the custom value + assert_eq!(example_struct_1.test_1, 500); + assert_eq!(example_struct_1.test_2, 100); + + // Assert that `test_3` is set to the default value (a) + assert_eq!(example_struct_1.test_3, "a".to_string()); + + // Case 2: test_2 missing + + // Example JSON string that doesn't contain `test_2` but has customized `test_1` and `test_3` + let json_data_2 = serde_json::json!({ + "test_1": 999, + "test_3": "test".to_string() + }); + + let example_struct_2: Example = serde_json::from_value(json_data_2).unwrap(); + + // Assert that`test_1` and `test_3` are set to the custom value + assert_eq!(example_struct_2.test_1, 999); + assert_eq!(example_struct_2.test_3, "test".to_string()); + + // Assert that `test_2` is set to the default value (42390) + assert_eq!(example_struct_2.test_2, 42390); +} + +#[test] +fn should_preserve_other_attributes() { + // Example JSON string that doesn't contain `test_3` but has customized `test_1` and `test_2` + let json_data = serde_json::json!({ + "test_1": 500, + "test_2": 100, + "test_3": "b".to_string() + }); + + let example_struct: Example = serde_json::from_value(json_data).unwrap(); + + // Assert that`test_2` and `test_3` have getters + assert_eq!(example_struct.test_1, 500); + assert_eq!(example_struct.test_2(), 100); + assert_eq!(example_struct.test_3(), &"b".to_string()); +} + +#[test] +fn test_struct_without_serialize() { + let json_data = serde_json::json!({ + "test_2": 123, + }); + + let example_without_serialize: ExampleWithoutSerialize = + serde_json::from_value(json_data).unwrap(); + + assert_eq!(example_without_serialize.test_1, 765); + assert_eq!(example_without_serialize.test_2, 123); +} + +#[test] +fn test_public_struct() { + let json_data = serde_json::json!({ + "test_1": 345, + }); + + let example_public: ExamplePublic = serde_json::from_value(json_data).unwrap(); + + assert_eq!(example_public.test_1, 345); + assert_eq!(example_public.test_2, 403); +}