Skip to content

Commit a48c009

Browse files
authored
Merge pull request #3310 from jarhodes314/feat/multi-dto-cleanup
fix: cleanup empty profiles
2 parents b843d72 + 696f713 commit a48c009

File tree

2 files changed

+169
-1
lines changed

2 files changed

+169
-1
lines changed

crates/common/tedge_config_macros/impl/src/query.rs

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use quote::quote;
1313
use quote::quote_spanned;
1414
use std::borrow::Cow;
1515
use std::collections::VecDeque;
16+
use std::iter::once;
1617
use syn::parse_quote;
1718
use syn::parse_quote_spanned;
1819
use syn::spanned::Spanned;
@@ -574,10 +575,12 @@ fn generate_field_accessor<'a>(
574575
FieldOrGroup::Field(_) => quote!(#ident),
575576
FieldOrGroup::Group(_) => quote!(#ident),
576577
FieldOrGroup::Multi(_) if exclude_parents => {
578+
// Interacting with TEdgeConfigReader - parents already included in value
577579
let field = id_gen.next_id(ident.span());
578580
quote_spanned!(ident.span()=> #ident.#method(#field.as_deref())?)
579581
}
580582
FieldOrGroup::Multi(_) => {
583+
// Interacting with TEdgeConfigDto - parents need to be supplied with try_get_mut
581584
let field = id_gen.next_id(ident.span());
582585
#[allow(unstable_name_collisions)]
583586
let parents = fields_so_far
@@ -591,6 +594,38 @@ fn generate_field_accessor<'a>(
591594
})
592595
}
593596

597+
fn generate_multi_dto_cleanup(fields: &VecDeque<&FieldOrGroup>) -> Vec<syn::Stmt> {
598+
let mut id_gen = SequentialIdGenerator::default();
599+
let mut all_idents = Vec::new();
600+
let mut fields_so_far = Vec::new();
601+
let mut result = Vec::new();
602+
for field in fields {
603+
let ident = field.ident();
604+
all_idents.push(ident);
605+
match field {
606+
FieldOrGroup::Multi(_) => {
607+
let field = id_gen.next_id(ident.span());
608+
#[allow(unstable_name_collisions)]
609+
let parents = all_idents
610+
.iter()
611+
.map(|id| id.to_string())
612+
.intersperse(".".to_owned())
613+
.collect::<String>();
614+
result.push(fields_so_far.iter().cloned().chain(once(quote_spanned!(ident.span()=> #ident.remove_if_empty(#field.as_deref())))).collect::<Vec<_>>());
615+
fields_so_far.push(
616+
quote_spanned!(ident.span()=> #ident.try_get_mut(#field.as_deref(), #parents)?),
617+
);
618+
}
619+
_ => fields_so_far.push(quote!(#ident)),
620+
}
621+
}
622+
result
623+
.into_iter()
624+
.rev()
625+
.map(|fields| parse_quote!(self.#(#fields).*;))
626+
.collect()
627+
}
628+
594629
fn generate_string_readers(paths: &[VecDeque<&FieldOrGroup>]) -> TokenStream {
595630
let enum_variants = paths.iter().map(enum_variant);
596631
let arms = paths
@@ -655,6 +690,7 @@ fn generate_string_writers(paths: &[VecDeque<&FieldOrGroup>]) -> TokenStream {
655690
.map(|(path, configuration_key)| {
656691
let read_segments = generate_field_accessor(path, "try_get", true);
657692
let write_segments = generate_field_accessor(path, "try_get_mut", false).collect::<Vec<_>>();
693+
let cleanup_stmts = generate_multi_dto_cleanup(path);
658694
let field = path
659695
.iter()
660696
.filter_map(|thing| thing.field())
@@ -687,7 +723,10 @@ fn generate_string_writers(paths: &[VecDeque<&FieldOrGroup>]) -> TokenStream {
687723
.map_err(|e| WriteError::ParseValue(Box::new(e)))?),
688724
},
689725
parse_quote_spanned! {ty.span()=>
690-
WritableKey::#match_variant => self.#(#write_segments).* = None,
726+
WritableKey::#match_variant => {
727+
self.#(#write_segments).* = None;
728+
#(#cleanup_stmts)*
729+
},
691730
},
692731
parse_quote_spanned! {ty.span()=>
693732
WritableKey::#match_variant => self.#(#write_segments).* = <#ty as AppendRemoveItem>::append(
@@ -1390,6 +1429,57 @@ mod tests {
13901429
);
13911430
}
13921431

1432+
#[test]
1433+
fn impl_try_unset_key_calls_multi_dto_cleanup() {
1434+
let input: crate::input::Configuration = parse_quote!(
1435+
#[tedge_config(multi)]
1436+
c8y: {
1437+
url: String,
1438+
1439+
#[tedge_config(multi)]
1440+
nested: {
1441+
field: bool,
1442+
}
1443+
},
1444+
1445+
sudo: {
1446+
enable: bool,
1447+
},
1448+
);
1449+
let paths = configuration_paths_from(&input.groups);
1450+
let writers = generate_string_writers(&paths);
1451+
let impl_dto_block = syn::parse2(writers).unwrap();
1452+
let impl_dto_block = retain_fn(impl_dto_block, "try_unset_key");
1453+
1454+
let expected = parse_quote! {
1455+
impl TEdgeConfigDto {
1456+
pub fn try_unset_key(&mut self, key: &WritableKey) -> Result<(), WriteError> {
1457+
match key {
1458+
WritableKey::C8yUrl(key0) => {
1459+
self.c8y.try_get_mut(key0.as_deref(), "c8y")?.url = None;
1460+
self.c8y.remove_if_empty(key0.as_deref());
1461+
}
1462+
WritableKey::C8yNestedField(key0, key1) => {
1463+
self.c8y.try_get_mut(key0.as_deref(), "c8y")?.nested.try_get_mut(key1.as_deref(), "c8y.nested")?.field = None;
1464+
// The fields should be removed from deepest to shallowest
1465+
self.c8y.try_get_mut(key0.as_deref(), "c8y")?.nested.remove_if_empty(key1.as_deref());
1466+
self.c8y.remove_if_empty(key0.as_deref());
1467+
}
1468+
WritableKey::SudoEnable => {
1469+
self.sudo.enable = None;
1470+
},
1471+
};
1472+
Ok(())
1473+
}
1474+
}
1475+
};
1476+
1477+
pretty_assertions::assert_eq!(
1478+
prettyplease::unparse(&parse_quote!(#impl_dto_block)),
1479+
prettyplease::unparse(&expected)
1480+
)
1481+
}
1482+
13931483
fn keys_enum_impl_block(config_keys: &(Vec<String>, Vec<ConfigurationKey>)) -> ItemImpl {
13941484
let generated = keys_enum(parse_quote!(ReadableKey), config_keys, "DOC FRAGMENT");
13951485
let generated_file: syn::File = syn::parse2(generated).unwrap();

crates/common/tedge_config_macros/src/multi.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,17 @@ impl<T: Default + PartialEq> MultiDto<T> {
160160
pub fn keys(&self) -> impl Iterator<Item = Option<&str>> {
161161
once(None).chain(self.profiles.keys().map(|k| k.0.as_str()).map(Some))
162162
}
163+
164+
/// Remove the key from the map if it is empty
165+
pub fn remove_if_empty(&mut self, key: Option<&str>) {
166+
if let Some(k) = key {
167+
if let Ok(val) = self.try_get(key, "") {
168+
if *val == T::default() {
169+
self.profiles.remove(k);
170+
}
171+
}
172+
}
173+
}
163174
}
164175

165176
impl<T> MultiReader<T> {
@@ -214,6 +225,7 @@ mod tests {
214225
use super::*;
215226
use serde::Deserialize;
216227
use serde_json::json;
228+
use tedge_config_macros_macro::define_tedge_config;
217229

218230
#[derive(Deserialize, Serialize, Debug, PartialEq, Eq)]
219231
struct TEdgeConfigDto {
@@ -446,4 +458,70 @@ mod tests {
446458
}
447459
);
448460
}
461+
462+
mod cleanup_on_unset {
463+
use super::*;
464+
use crate::*;
465+
466+
define_tedge_config! {
467+
#[tedge_config(multi)]
468+
c8y: {
469+
url: String,
470+
availability: {
471+
interval: String,
472+
},
473+
}
474+
}
475+
476+
#[test]
477+
fn multi_dto_is_cleaned_up_if_default_value() {
478+
let mut config: TEdgeConfigDto =
479+
toml::from_str("c8y.profiles.test.url = \"example.com\"").unwrap();
480+
config
481+
.try_unset_key(&WritableKey::C8yUrl(Some("test".into())))
482+
.unwrap();
483+
assert_eq!(config.c8y.profiles.len(), 0);
484+
}
485+
486+
#[test]
487+
fn multi_dto_is_not_cleaned_up_if_not_default_value() {
488+
let mut config: TEdgeConfigDto = toml::from_str(
489+
"[c8y.profiles.test]\nurl = \"example.com\"\navailability.interval = \"60m\"",
490+
)
491+
.unwrap();
492+
config
493+
.try_unset_key(&WritableKey::C8yUrl(Some("test".into())))
494+
.unwrap();
495+
assert_eq!(config.c8y.profiles.len(), 1);
496+
}
497+
498+
#[derive(Debug, thiserror::Error)]
499+
enum ReadError {
500+
#[error(transparent)]
501+
ConfigNotSet(#[from] ConfigNotSet),
502+
#[error(transparent)]
503+
Multi(#[from] MultiError),
504+
}
505+
#[allow(unused)]
506+
trait AppendRemoveItem {
507+
type Item;
508+
fn append(
509+
current_value: Option<Self::Item>,
510+
new_value: Self::Item,
511+
) -> Option<Self::Item>;
512+
fn remove(
513+
current_value: Option<Self::Item>,
514+
remove_value: Self::Item,
515+
) -> Option<Self::Item>;
516+
}
517+
impl AppendRemoveItem for String {
518+
type Item = Self;
519+
fn append(_: Option<Self::Item>, _: Self::Item) -> Option<Self::Item> {
520+
unimplemented!()
521+
}
522+
fn remove(_: Option<Self::Item>, _: Self::Item) -> Option<Self::Item> {
523+
unimplemented!()
524+
}
525+
}
526+
}
449527
}

0 commit comments

Comments
 (0)