From 8d945b1717140a8d572412efaaac70a756418edd Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Wed, 8 Jan 2025 18:45:48 +0000 Subject: [PATCH 1/3] refactor: support writable device ids --- .../src/tedge_config_cli/tedge_config.rs | 36 +- .../tedge_config_macros/examples/macro.rs | 2 +- .../tedge_config_macros/impl/src/dto.rs | 148 ++++++- .../impl/src/input/parse.rs | 3 +- .../impl/src/input/validate.rs | 58 +++ .../tedge_config_macros/impl/src/query.rs | 56 ++- .../tedge_config_macros/impl/src/reader.rs | 405 ++++++++++++++++-- 7 files changed, 636 insertions(+), 72 deletions(-) diff --git a/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs b/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs index 6a0a59d592d..b02e6774f15 100644 --- a/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs +++ b/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs @@ -428,15 +428,9 @@ define_tedge_config! { device: { /// Identifier of the device within the fleet. It must be globally /// unique and is derived from the device certificate. - #[tedge_config(readonly( - write_error = "\ - The device id is read from the device certificate and cannot be set directly.\n\ - To set 'device.id' to some , you can use `tedge cert create --device-id `.", - function = "device_id", - ))] + #[tedge_config(reader(function = "device_id"))] #[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")] #[tedge_config(note = "This setting is derived from the device certificate and is therefore read only.")] - #[tedge_config(reader(private))] #[doku(as = "String")] id: Result, @@ -489,14 +483,10 @@ define_tedge_config! { device: { /// Identifier of the device within the fleet. It must be globally /// unique and is derived from the device certificate. - #[tedge_config(readonly( - write_error = "\ - The device id is read from the device certificate and cannot be set directly.\n\ - To set 'device.id' to some , you can use `tedge cert create --device-id `.", - function = "c8y_device_id", - ))] + #[tedge_config(reader(function = "c8y_device_id"))] + // TODO make this work + // #[tedge_config(default(from_optional_key = "device.id"))] #[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")] - #[tedge_config(note = "This setting is derived from the device certificate and is therefore read only.")] #[doku(as = "String")] id: Result, @@ -1355,26 +1345,32 @@ fn default_http_bind_address(dto: &TEdgeConfigDto) -> IpAddr { fn device_id_from_cert(cert_path: &Utf8Path) -> Result { let pem = PemCertificate::from_pem_file(cert_path) - .map_err(|err| cert_error_into_config_error(ReadOnlyKey::DeviceId.to_cow_str(), err))?; + .map_err(|err| cert_error_into_config_error(ReadableKey::DeviceId.to_cow_str(), err))?; let device_id = pem .subject_common_name() - .map_err(|err| cert_error_into_config_error(ReadOnlyKey::DeviceId.to_cow_str(), err))?; + .map_err(|err| cert_error_into_config_error(ReadableKey::DeviceId.to_cow_str(), err))?; Ok(device_id) } -fn device_id(device: &TEdgeConfigReaderDevice) -> Result { +fn device_id( + device: &TEdgeConfigReaderDevice, + dto_value: &OptionalConfig, +) -> Result { device_id_from_cert(&device.cert_path) } -fn c8y_device_id(device: &TEdgeConfigReaderC8yDevice) -> Result { +fn c8y_device_id( + device: &TEdgeConfigReaderC8yDevice, + dto_value: &OptionalConfig, +) -> Result { device_id_from_cert(&device.cert_path) } -fn az_device_id(device: &TEdgeConfigReaderAzDevice) -> Result { +fn az_device_id(device: &TEdgeConfigReaderAzDevice, _: &()) -> Result { device_id_from_cert(&device.cert_path) } -fn aws_device_id(device: &TEdgeConfigReaderAwsDevice) -> Result { +fn aws_device_id(device: &TEdgeConfigReaderAwsDevice, _: &()) -> Result { device_id_from_cert(&device.cert_path) } diff --git a/crates/common/tedge_config_macros/examples/macro.rs b/crates/common/tedge_config_macros/examples/macro.rs index 6162aa735f1..dc283385fa5 100644 --- a/crates/common/tedge_config_macros/examples/macro.rs +++ b/crates/common/tedge_config_macros/examples/macro.rs @@ -189,7 +189,7 @@ define_tedge_config! { } } -fn device_id(_reader: &TEdgeConfigReaderDevice) -> Result { +fn device_id(_reader: &TEdgeConfigReaderDevice, _: &()) -> Result { Ok("dummy-device-id".to_owned()) } diff --git a/crates/common/tedge_config_macros/impl/src/dto.rs b/crates/common/tedge_config_macros/impl/src/dto.rs index 4fbe7e661f3..6e6c63b6ddc 100644 --- a/crates/common/tedge_config_macros/impl/src/dto.rs +++ b/crates/common/tedge_config_macros/impl/src/dto.rs @@ -4,6 +4,7 @@ use quote::quote_spanned; use syn::parse_quote_spanned; use syn::spanned::Spanned; +use crate::error::extract_type_from_result; use crate::input::FieldOrGroup; use crate::prefixed_type_name; @@ -21,7 +22,17 @@ pub fn generate( for item in items { match item { FieldOrGroup::Field(field) => { - if !field.dto().skip && field.read_only().is_none() { + if field.reader_function().is_some() { + let ty = match extract_type_from_result(field.ty()) { + Some((ok, _err)) => ok, + None => field.ty(), + }; + idents.push(field.ident()); + tys.push(parse_quote_spanned!(ty.span() => Option<#ty>)); + sub_dtos.push(None); + preserved_attrs.push(field.attrs().iter().filter(is_preserved).collect()); + extra_attrs.push(quote! {}); + } else if !field.dto().skip && field.read_only().is_none() { idents.push(field.ident()); tys.push({ let ty = field.ty(); @@ -106,7 +117,11 @@ fn is_preserved(attr: &&syn::Attribute) -> bool { #[cfg(test)] mod tests { + use proc_macro2::Span; use syn::parse_quote; + use syn::Ident; + use syn::Item; + use syn::ItemStruct; use super::*; @@ -137,4 +152,135 @@ mod tests { #[unknown_attribute = "some value"] ))) } + + #[test] + fn dto_is_generated() { + let input: crate::input::Configuration = parse_quote!( + c8y: { + url: String, + }, + sudo: { + enable: bool, + }, + ); + + let generated = generate_test_dto(&input); + let expected = parse_quote! { + #[derive(Debug, Default, ::serde::Deserialize, ::serde::Serialize, PartialEq)] + #[non_exhaustive] + pub struct TEdgeConfigDto { + #[serde(default)] + #[serde(skip_serializing_if = "TEdgeConfigDtoC8y::is_default")] + pub c8y: TEdgeConfigDtoC8y, + #[serde(default)] + #[serde(skip_serializing_if = "TEdgeConfigDtoSudo::is_default")] + pub sudo: TEdgeConfigDtoSudo, + } + + impl TEdgeConfigDto { + #[allow(unused)] + fn is_default(&self) -> bool { + self == &Self::default() + } + } + + #[derive(Debug, Default, ::serde::Deserialize, ::serde::Serialize, PartialEq)] + #[non_exhaustive] + pub struct TEdgeConfigDtoC8y { + pub url: Option, + } + + impl TEdgeConfigDtoC8y { + #[allow(unused)] + fn is_default(&self) -> bool { + self == &Self::default() + } + } + + #[derive(Debug, Default, ::serde::Deserialize, ::serde::Serialize, PartialEq)] + #[non_exhaustive] + pub struct TEdgeConfigDtoSudo { + pub enable: Option, + } + + impl TEdgeConfigDtoSudo { + #[allow(unused)] + fn is_default(&self) -> bool { + self == &Self::default() + } + } + }; + + assert_eq(&generated, &expected); + } + + #[test] + fn ok_type_is_extracted_from_reader_function_if_relevant() { + let input: crate::input::Configuration = parse_quote!( + device: { + #[tedge_config(reader(function = "c8y_device_id"))] + id: Result, + } + ); + + let mut generated = generate_test_dto(&input); + generated + .items + .retain(only_struct_named("TEdgeConfigDtoDevice")); + + let expected = parse_quote! { + #[derive(Debug, Default, ::serde::Deserialize, ::serde::Serialize, PartialEq)] + #[non_exhaustive] + pub struct TEdgeConfigDtoDevice { + pub id: Option, + } + }; + + assert_eq(&generated, &expected); + } + + #[test] + fn reader_function_type_is_used_verbatim_in_dto_if_not_result() { + let input: crate::input::Configuration = parse_quote!( + device: { + #[tedge_config(reader(function = "c8y_device_id"))] + id: String, + } + ); + + let mut generated = generate_test_dto(&input); + generated + .items + .retain(only_struct_named("TEdgeConfigDtoDevice")); + + let expected = parse_quote! { + #[derive(Debug, Default, ::serde::Deserialize, ::serde::Serialize, PartialEq)] + #[non_exhaustive] + pub struct TEdgeConfigDtoDevice { + pub id: Option, + } + }; + + assert_eq(&generated, &expected); + } + + fn generate_test_dto(input: &crate::input::Configuration) -> syn::File { + let tokens = super::generate( + Ident::new("TEdgeConfigDto", Span::call_site()), + &input.groups, + "", + ); + syn::parse2(tokens).unwrap() + } + + fn assert_eq(actual: &syn::File, expected: &syn::File) { + pretty_assertions::assert_eq!( + prettyplease::unparse(&actual), + prettyplease::unparse(&expected), + ) + } + + fn only_struct_named<'a>(target: &'a str) -> impl Fn(&Item) -> bool + 'a { + move |i| matches!(i, Item::Struct(ItemStruct { ident, .. }) if ident == target) + } } diff --git a/crates/common/tedge_config_macros/impl/src/input/parse.rs b/crates/common/tedge_config_macros/impl/src/input/parse.rs index 1e0a1e8dfa2..bd36d3f5761 100644 --- a/crates/common/tedge_config_macros/impl/src/input/parse.rs +++ b/crates/common/tedge_config_macros/impl/src/input/parse.rs @@ -5,7 +5,7 @@ // FIXME: if let can be simplified with `.unwrap_or_default()` // for all `#[darling(default)]` -#![allow(clippy::manual_unwrap_or_default)] +// #![allow(clippy::manual_unwrap_or_default)] use darling::util::SpannedValue; use darling::FromAttributes; @@ -198,6 +198,7 @@ pub struct FieldDtoSettings { pub struct ReaderSettings { #[darling(default)] pub private: bool, + pub function: Option, #[darling(default)] pub skip: bool, } diff --git a/crates/common/tedge_config_macros/impl/src/input/validate.rs b/crates/common/tedge_config_macros/impl/src/input/validate.rs index b9c3667739a..856b5be7c7f 100644 --- a/crates/common/tedge_config_macros/impl/src/input/validate.rs +++ b/crates/common/tedge_config_macros/impl/src/input/validate.rs @@ -9,6 +9,7 @@ use syn::parse_quote_spanned; use syn::spanned::Spanned; use crate::error::combine_errors; +use crate::error::extract_type_from_result; use crate::optional_error::OptionalError; use crate::optional_error::SynResultExt; use crate::reader::PathItem; @@ -207,6 +208,40 @@ impl ReadOnlyField { } } +impl ReadWriteField { + pub fn lazy_reader_name(&self, parents: &[PathItem]) -> syn::Ident { + format_ident!( + "LazyReader{}{}", + parents + .iter() + .filter_map(PathItem::as_static) + .map(|p| p.to_string().to_upper_camel_case()) + .collect::>() + .join(""), + self.rename() + .map(<_>::to_owned) + .unwrap_or_else(|| self.ident.to_string()) + .to_upper_camel_case() + ) + } + + pub fn parent_name(&self, parents: &[PathItem]) -> syn::Ident { + format_ident!( + "TEdgeConfigReader{}", + parents + .iter() + .filter_map(PathItem::as_static) + .map(|p| p.to_string().to_upper_camel_case()) + .collect::>() + .join(""), + ) + } + + pub fn rename(&self) -> Option<&str> { + Some(self.rename.as_ref()?.as_str()) + } +} + #[derive(Debug)] pub struct ReadWriteField { pub attrs: Vec, @@ -301,6 +336,21 @@ impl ConfigurableField { } } + pub fn reader_function(&self) -> Option<(&syn::Path, &ReadWriteField)> { + match self { + Self::ReadWrite( + field @ ReadWriteField { + reader: + ReaderSettings { + function: Some(f), .. + }, + .. + }, + ) => Some((f, field)), + _ => None, + } + } + pub fn deprecated_keys(&self) -> impl Iterator { let keys = match self { Self::ReadOnly(field) => &field.deprecated_keys, @@ -357,6 +407,14 @@ impl TryFrom for ConfigurableField { .push(parse_quote_spanned!(span=> #[serde(rename = #literal)])) } + if value.reader.function.is_some() && value.from.is_none() { + value.from = Some( + extract_type_from_result(&value.ty) + .map_or(&value.ty, |tys| tys.0) + .clone(), + ); + } + for name in value.deprecated_names { let name_str = name.as_str(); if name.contains('.') { diff --git a/crates/common/tedge_config_macros/impl/src/query.rs b/crates/common/tedge_config_macros/impl/src/query.rs index edade35c96b..119c87a4a94 100644 --- a/crates/common/tedge_config_macros/impl/src/query.rs +++ b/crates/common/tedge_config_macros/impl/src/query.rs @@ -642,7 +642,7 @@ fn generate_string_readers(paths: &[VecDeque<&FieldOrGroup>]) -> TokenStream { parent_segments.remove(parent_segments.len() - 1); let to_string = quote_spanned!(field.ty().span()=> .to_string()); let match_variant = configuration_key.match_read_write; - if field.read_only().is_some() { + if field.read_only().is_some() || field.reader_function().is_some() { if extract_type_from_result(field.ty()).is_some() { parse_quote! { ReadableKey::#match_variant => Ok(self.#(#segments).*()?#to_string), @@ -698,16 +698,21 @@ fn generate_string_writers(paths: &[VecDeque<&FieldOrGroup>]) -> TokenStream { .unwrap(); let match_variant = configuration_key.match_read_write; - let ty = field.ty(); + let ty = if field.reader_function().is_some() { + extract_type_from_result(field.ty()).map(|tys| tys.0).unwrap_or(field.ty()) + } else { + field.ty() + }; + let parse_as = field.from().unwrap_or(field.ty()); let parse = quote_spanned! {parse_as.span()=> parse::<#parse_as>() }; let convert_to_field_ty = quote_spanned! {ty.span()=> map(<#ty>::from)}; - let current_value = if field.read_only().is_some() { + let current_value = if field.read_only().is_some() || field.reader_function().is_some() { if extract_type_from_result(field.ty()).is_some() { - quote_spanned! {ty.span()=> reader.#(#read_segments).*.try_read(reader).ok()} + quote_spanned! {ty.span()=> reader.#(#read_segments).*().ok().cloned()} } else { - quote_spanned! {ty.span()=> Some(reader.#(#read_segments).*.read(reader))} + quote_spanned! {ty.span()=> Some(reader.#(#read_segments).*())} } } else if field.has_guaranteed_default() { quote_spanned! {ty.span()=> Some(reader.#(#read_segments).*.to_owned())} @@ -1480,6 +1485,47 @@ mod tests { ) } + #[test] + fn impl_try_append_calls_method_for_current_value() { + let input: crate::input::Configuration = parse_quote!( + #[tedge_config(multi)] + c8y: { + device: { + #[tedge_config(reader(function = "device_id"))] + id: String, + }, + } + ); + let paths = configuration_paths_from(&input.groups); + let writers = generate_string_writers(&paths); + let impl_dto_block = syn::parse2(writers).unwrap(); + let impl_dto_block = retain_fn(impl_dto_block, "try_append_str"); + + let expected = parse_quote! { + impl TEdgeConfigDto { + pub fn try_append_str(&mut self, reader: &TEdgeConfigReader, key: &WritableKey, value: &str) -> Result<(), WriteError> { + match key { + WritableKey::C8yDeviceId(key0) => { + self.c8y.try_get_mut(key0.as_deref(), "c8y")?.device.id = ::append( + Some(reader.c8y.try_get(key0.as_deref())?.device.id()), + value + .parse::() + .map(::from) + .map_err(|e| WriteError::ParseValue(Box::new(e)))?, + ); + } + }; + Ok(()) + } + } + }; + + pretty_assertions::assert_eq!( + prettyplease::unparse(&parse_quote!(#impl_dto_block)), + prettyplease::unparse(&expected) + ) + } + fn keys_enum_impl_block(config_keys: &(Vec, Vec)) -> ItemImpl { let generated = keys_enum(parse_quote!(ReadableKey), config_keys, "DOC FRAGMENT"); let generated_file: syn::File = syn::parse2(generated).unwrap(); diff --git a/crates/common/tedge_config_macros/impl/src/reader.rs b/crates/common/tedge_config_macros/impl/src/reader.rs index 782cad20c8b..894f9d19036 100644 --- a/crates/common/tedge_config_macros/impl/src/reader.rs +++ b/crates/common/tedge_config_macros/impl/src/reader.rs @@ -57,8 +57,32 @@ fn generate_structs( let ty = field.ty(); attrs.push(field.attrs().to_vec()); idents.push(field.ident()); - if field.is_optional() { + if let Some((function, field)) = field.reader_function() { + let name = field.lazy_reader_name(&parents); + let parent_ty = field.parent_name(&parents); + tys.push(parse_quote_spanned!(ty.span()=> #name)); + let dto_ty: syn::Type = match extract_type_from_result(&field.ty) { + Some((ok, _err)) => parse_quote!(OptionalConfig<#ok>), + None => { + let ty = &field.ty; + parse_quote!(OptionalConfig<#ty>) + } + }; + lazy_readers.push(( + name, + &field.ty, + function, + parent_ty, + field.ident.clone(), + dto_ty.clone(), + )); + vis.push(parse_quote!()); + } else if field.is_optional() { tys.push(parse_quote_spanned!(ty.span()=> OptionalConfig<#ty>)); + vis.push(match field.reader().private { + true => parse_quote!(), + false => parse_quote!(pub), + }); } else if let Some(field) = field.read_only() { let name = field.lazy_reader_name(&parents); let parent_ty = field.parent_name(&parents); @@ -69,15 +93,17 @@ fn generate_structs( &field.readonly.function, parent_ty, field.ident.clone(), + parse_quote!(()), )); + vis.push(parse_quote!()); } else { tys.push(ty.to_owned()); + vis.push(match field.reader().private { + true => parse_quote!(), + false => parse_quote!(pub), + }); } sub_readers.push(None); - vis.push(match field.reader().private { - true => parse_quote!(), - false => parse_quote!(pub), - }); } FieldOrGroup::Multi(group) if !group.reader.skip => { let sub_reader_name = prefixed_type_name(name, group); @@ -125,36 +151,37 @@ fn generate_structs( } } - let lazy_reader_impls = - lazy_readers - .iter() - .map(|(name, ty, function, parent_ty, id)| -> syn::ItemImpl { - if let Some((ok, err)) = extract_type_from_result(ty) { - parse_quote_spanned! {name.span()=> - impl #parent_ty { - pub fn #id(&self) -> Result<&#ok, #err> { - self.#id.0.get_or_try_init(|| #function(self)) - } + let lazy_reader_impls = lazy_readers.iter().map( + |(name, ty, function, parent_ty, id, _dto_ty)| -> syn::ItemImpl { + if let Some((ok, err)) = extract_type_from_result(ty) { + parse_quote_spanned! {name.span()=> + impl #parent_ty { + pub fn #id(&self) -> Result<&#ok, #err> { + self.#id.0.get_or_try_init(|| #function(self, &self.#id.1)) } } - } else { - parse_quote_spanned! {name.span()=> - impl #parent_ty { - pub fn #id(&self) -> &#ty { - self.#id.0.get_or_init(|| #function(self)) - } + } + } else { + parse_quote_spanned! {name.span()=> + impl #parent_ty { + pub fn #id(&self) -> &#ty { + self.#id.0.get_or_init(|| #function(self, &self.#id.1)) } } } - }); + } + }, + ); - let (lr_names, lr_tys): (Vec<_>, Vec<_>) = lazy_readers + let (lr_names, lr_tys, lr_dto_tys): (Vec<_>, Vec<_>, Vec<_>) = lazy_readers .iter() - .map(|(name, ty, _, _, _)| match extract_type_from_result(ty) { - Some((ok, _err)) => (name, ok), - None => (name, *ty), - }) - .unzip(); + .map( + |(name, ty, _, _, _, dto_ty)| match extract_type_from_result(ty) { + Some((ok, _err)) => (name, ok, dto_ty), + None => (name, *ty, dto_ty), + }, + ) + .multiunzip(); let doc_comment_attr = (!doc_comment.is_empty()).then(|| quote_spanned!(name.span()=> #[doc = #doc_comment])); @@ -170,9 +197,9 @@ fn generate_structs( } #( - #[derive(::serde::Serialize, Clone, Debug, Default)] - #[serde(into = "()")] - pub struct #lr_names(::once_cell::sync::OnceCell<#lr_tys>); + #[derive(::serde::Serialize, Clone, Debug)] + #[serde(into = "()")] // Just a hack to support serialization, required for doku + pub struct #lr_names(::once_cell::sync::OnceCell<#lr_tys>, #lr_dto_tys); impl From<#lr_names> for () { fn from(_: #lr_names) {} @@ -306,8 +333,8 @@ fn reader_value_for_field<'a>( parse_quote!(ReadableKey::#ident(#(#args.map(<_>::to_owned)),*).to_cow_str()) }; let read_path = read_field(parents); - match &rw_field.default { - FieldDefault::None => quote! { + let value = match &rw_field.default { + FieldDefault::None => quote_spanned! {rw_field.ident.span()=> match &dto.#(#read_path).*.#name { None => OptionalConfig::Empty(#key), Some(value) => OptionalConfig::Present { value: value.clone(), key: #key }, @@ -345,15 +372,20 @@ fn reader_value_for_field<'a>( observed_keys, )?; - let (default, value) = - if matches!(&rw_field.default, FieldDefault::FromOptionalKey(_)) { - ( - quote!(#default.map(|v| v.into())), - quote!(OptionalConfig::Present { value: value.clone(), key: #key }), - ) - } else { - (quote!(#default.into()), quote!(value.clone())) - }; + let (default, value) = if matches!( + &rw_field.default, + FieldDefault::FromOptionalKey(_) + ) { + ( + quote_spanned!(default_key.span()=> #default.map(|v| v.into())), + quote_spanned!(default_key.span()=> OptionalConfig::Present { value: value.clone(), key: #key }), + ) + } else { + ( + quote_spanned!(default_key.span()=> #default.into()), + quote_spanned!(default_key.span()=> value.clone()), + ) + }; quote_spanned! {name.span()=> match &dto.#(#read_path).*.#name { @@ -386,12 +418,20 @@ fn reader_value_for_field<'a>( Some(value) => value.clone(), } }, + }; + if field.reader_function().is_some() { + let name = rw_field.lazy_reader_name(parents); + quote_spanned! {rw_field.ident.span()=> + #name(<_>::default(), #value) + } + } else { + value } } ConfigurableField::ReadOnly(field) => { let name = field.lazy_reader_name(parents); - quote! { - #name::default() + quote_spanned! {field.ident.span()=> + #name(<_>::default(), ()) } } }) @@ -589,6 +629,9 @@ fn generate_conversions( mod tests { use super::*; use syn::parse_quote; + use syn::Item; + use syn::ItemImpl; + use syn::ItemStruct; #[test] fn from_optional_key_reuses_multi_fields() { @@ -725,4 +768,278 @@ mod tests { prettyplease::unparse(&expected) ) } + + #[test] + fn generate_structs_generates_getter_for_readonly_value() { + let input: crate::input::Configuration = parse_quote!( + device: { + #[tedge_config(readonly( + write_error = "\ + The device id is read from the device certificate and cannot be set directly.\n\ + To set 'device.id' to some , you can use `tedge cert create --device-id `.", + function = "device_id", + ))] + id: String, + }, + ); + let actual = generate_structs( + &parse_quote!(TEdgeConfigReader), + &input.groups, + Vec::new(), + "", + ) + .unwrap(); + let file: syn::File = syn::parse2(actual).unwrap(); + + let expected = parse_quote! { + #[derive(::doku::Document, ::serde::Serialize, Debug, Clone)] + #[non_exhaustive] + pub struct TEdgeConfigReader { + pub device: TEdgeConfigReaderDevice, + } + #[derive(::doku::Document, ::serde::Serialize, Debug, Clone)] + #[non_exhaustive] + pub struct TEdgeConfigReaderDevice { + id: LazyReaderDeviceId, + } + #[derive(::serde::Serialize, Clone, Debug)] + #[serde(into = "()")] + pub struct LazyReaderDeviceId(::once_cell::sync::OnceCell, ()); + impl From for () { + fn from(_: LazyReaderDeviceId) {} + } + impl TEdgeConfigReaderDevice { + pub fn id(&self) -> &String { + self.id.0.get_or_init(|| device_id(self, &self.id.1)) + } + } + }; + + pretty_assertions::assert_eq!( + prettyplease::unparse(&file), + prettyplease::unparse(&expected) + ) + } + + #[test] + fn generate_structs_generates_getter_for_reader_function_value() { + let input: crate::input::Configuration = parse_quote!( + device: { + #[tedge_config(reader(function = "device_id"))] + id: String, + }, + ); + let actual = generate_structs( + &parse_quote!(TEdgeConfigReader), + &input.groups, + Vec::new(), + "", + ) + .unwrap(); + let mut file: syn::File = syn::parse2(actual).unwrap(); + let target: syn::Type = parse_quote!(TEdgeConfigReaderDevice); + file.items + .retain(|i| matches!(i, Item::Impl(ItemImpl { self_ty, .. }) if **self_ty == target)); + + let expected = parse_quote! { + impl TEdgeConfigReaderDevice { + pub fn id(&self) -> &String { + self.id.0.get_or_init(|| device_id(self, &self.id.1)) + } + } + }; + + pretty_assertions::assert_eq!( + prettyplease::unparse(&file), + prettyplease::unparse(&expected) + ) + } + + #[test] + fn fields_are_public_only_if_directly_readable() { + let input: crate::input::Configuration = parse_quote!( + test: { + #[tedge_config(reader(function = "device_id"))] + read_via_function: String, + #[tedge_config(readonly(write_error = "TODO", function="device_id"))] + readonly: String, + #[tedge_config(default(value = "test"))] + with_default: String, + optional: String, + }, + ); + let actual = generate_structs( + &parse_quote!(TEdgeConfigReader), + &input.groups, + Vec::new(), + "", + ) + .unwrap(); + let mut file: syn::File = syn::parse2(actual).unwrap(); + file.items.retain(|s| matches!(s, Item::Struct(ItemStruct { ident, ..}) if ident == "TEdgeConfigReaderTest")); + + let expected = parse_quote! { + #[derive(::doku::Document, ::serde::Serialize, Debug, Clone)] + #[non_exhaustive] + pub struct TEdgeConfigReaderTest { + read_via_function: LazyReaderTestReadViaFunction, + readonly: LazyReaderTestReadonly, + pub with_default: String, + pub optional: OptionalConfig, + } + }; + + pretty_assertions::assert_eq!( + prettyplease::unparse(&file), + prettyplease::unparse(&expected) + ) + } + + #[test] + fn default_values_do_stuff() { + let input: crate::input::Configuration = parse_quote!( + c8y: { + #[tedge_config(default(from_optional_key = "c8y.url"))] + http: String, + url: String, + }, + ); + let actual = generate_conversions( + &parse_quote!(TEdgeConfigReader), + &input.groups, + Vec::new(), + &input.groups, + ) + .unwrap(); + let file: syn::File = syn::parse2(actual).unwrap(); + + let expected = parse_quote! { + impl TEdgeConfigReader { + #[allow(unused, clippy::clone_on_copy, clippy::useless_conversion)] + #[automatically_derived] + /// Converts the provided [TEdgeConfigDto] into a reader + pub fn from_dto(dto: &TEdgeConfigDto, location: &TEdgeConfigLocation) -> Self { + Self { + c8y: TEdgeConfigReaderC8y::from_dto(dto, location), + } + } + } + impl TEdgeConfigReaderC8y { + #[allow(unused, clippy::clone_on_copy, clippy::useless_conversion)] + #[automatically_derived] + /// Converts the provided [TEdgeConfigDto] into a reader + pub fn from_dto(dto: &TEdgeConfigDto, location: &TEdgeConfigLocation) -> Self { + Self { + http: match &dto.c8y.http { + Some(value) => { + OptionalConfig::Present { + value: value.clone(), + key: ReadableKey::C8yHttp.to_cow_str(), + } + } + None => { + match &dto.c8y.url { + None => OptionalConfig::Empty(ReadableKey::C8yUrl.to_cow_str()), + Some(value) => { + OptionalConfig::Present { + value: value.clone(), + key: ReadableKey::C8yUrl.to_cow_str(), + } + } + } + .map(|v| v.into()) + } + }, + url: match &dto.c8y.url { + None => OptionalConfig::Empty(ReadableKey::C8yUrl.to_cow_str()), + Some(value) => { + OptionalConfig::Present { + value: value.clone(), + key: ReadableKey::C8yUrl.to_cow_str(), + } + } + }, + } + } + } + }; + + pretty_assertions::assert_eq!( + prettyplease::unparse(&file), + prettyplease::unparse(&expected) + ) + } + + #[test] + fn default_values_do_stuff2() { + let input: crate::input::Configuration = parse_quote!( + device: { + #[tedge_config(reader(function = "device_id"))] + id: Result, + }, + c8y: { + device: { + #[tedge_config(default(from_optional_key = "device.id"))] + #[tedge_config(reader(function = "c8y_device_id"))] + id: Result, + }, + }, + ); + let actual = generate_conversions( + &parse_quote!(TEdgeConfigReader), + &input.groups, + Vec::new(), + &input.groups, + ) + .unwrap(); + let mut file: syn::File = syn::parse2(actual).unwrap(); + let target: syn::Type = parse_quote!(TEdgeConfigReaderC8yDevice); + file.items + .retain(|i| matches!(i, Item::Impl(ItemImpl { self_ty, ..}) if **self_ty == target)); + + let expected = parse_quote! { + impl TEdgeConfigReaderC8yDevice { + #[allow(unused, clippy::clone_on_copy, clippy::useless_conversion)] + #[automatically_derived] + /// Converts the provided [TEdgeConfigDto] into a reader + pub fn from_dto(dto: &TEdgeConfigDto, location: &TEdgeConfigLocation) -> Self { + Self { + id: LazyReaderC8yDeviceId( + <_>::default(), + match &dto.c8y.device.id { + Some(value) => { + OptionalConfig::Present { + value: value.clone(), + key: ReadableKey::C8yDeviceId.to_cow_str(), + } + } + None => { + LazyReaderDeviceId( + <_>::default(), + match &dto.device.id { + None => { + OptionalConfig::Empty(ReadableKey::DeviceId.to_cow_str()) + } + Some(value) => { + OptionalConfig::Present { + value: value.clone(), + key: ReadableKey::DeviceId.to_cow_str(), + } + } + }, + ) + .map(|v| v.into()) + } + }, + ), + } + } + } + }; + + pretty_assertions::assert_eq!( + prettyplease::unparse(&file), + prettyplease::unparse(&expected) + ) + } } From d75841957acdab4ec755fb0c6779dafd462136ad Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Thu, 9 Jan 2025 16:26:51 +0000 Subject: [PATCH 2/3] Make defaults work --- .../src/tedge_config_cli/tedge_config.rs | 15 +- crates/common/tedge_config_macros/Cargo.toml | 1 + .../tedge_config_macros/examples/multi.rs | 6 +- .../examples/reader_function.rs | 147 ++++++++++++++++++ .../tedge_config_macros/impl/src/reader.rs | 28 ++-- crates/core/tedge/src/cli/upload/mod.rs | 3 +- 6 files changed, 178 insertions(+), 22 deletions(-) create mode 100644 crates/common/tedge_config_macros/examples/reader_function.rs diff --git a/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs b/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs index b02e6774f15..106eff169cd 100644 --- a/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs +++ b/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs @@ -484,8 +484,7 @@ define_tedge_config! { /// Identifier of the device within the fleet. It must be globally /// unique and is derived from the device certificate. #[tedge_config(reader(function = "c8y_device_id"))] - // TODO make this work - // #[tedge_config(default(from_optional_key = "device.id"))] + #[tedge_config(default(from_optional_key = "device.id"))] #[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")] #[doku(as = "String")] id: Result, @@ -1356,14 +1355,20 @@ fn device_id( device: &TEdgeConfigReaderDevice, dto_value: &OptionalConfig, ) -> Result { - device_id_from_cert(&device.cert_path) + match dto_value.or_none() { + Some(id) => Ok(id.clone()), + None => device_id_from_cert(&device.cert_path) + } } fn c8y_device_id( - device: &TEdgeConfigReaderC8yDevice, + c8y_device: &TEdgeConfigReaderC8yDevice, dto_value: &OptionalConfig, ) -> Result { - device_id_from_cert(&device.cert_path) + match dto_value.or_none() { + Some(id) => Ok(id.clone()), + None => device_id_from_cert(&c8y_device.cert_path) + } } fn az_device_id(device: &TEdgeConfigReaderAzDevice, _: &()) -> Result { diff --git a/crates/common/tedge_config_macros/Cargo.toml b/crates/common/tedge_config_macros/Cargo.toml index afd5dca15c1..27179fccade 100644 --- a/crates/common/tedge_config_macros/Cargo.toml +++ b/crates/common/tedge_config_macros/Cargo.toml @@ -21,6 +21,7 @@ tracing = { workspace = true } url = { workspace = true } [dev-dependencies] +certificate = { workspace = true, features = ["reqwest"] } serde = { workspace = true, features = ["rc"] } serde_json = { workspace = true } toml = { workspace = true } diff --git a/crates/common/tedge_config_macros/examples/multi.rs b/crates/common/tedge_config_macros/examples/multi.rs index 7414c39ed98..71d0ae90e8b 100644 --- a/crates/common/tedge_config_macros/examples/multi.rs +++ b/crates/common/tedge_config_macros/examples/multi.rs @@ -111,14 +111,14 @@ fn main() { keys, [ "c8y.http", - "c8y.smartrest.use_operation_id", - "c8y.url", "c8y.profiles.cloud.http", "c8y.profiles.cloud.smartrest.use_operation_id", "c8y.profiles.cloud.url", "c8y.profiles.edge.http", "c8y.profiles.edge.smartrest.use_operation_id", - "c8y.profiles.edge.url" + "c8y.profiles.edge.url", + "c8y.smartrest.use_operation_id", + "c8y.url", ] ); } diff --git a/crates/common/tedge_config_macros/examples/reader_function.rs b/crates/common/tedge_config_macros/examples/reader_function.rs new file mode 100644 index 00000000000..90d5ca8f2cc --- /dev/null +++ b/crates/common/tedge_config_macros/examples/reader_function.rs @@ -0,0 +1,147 @@ +use camino::{Utf8Path, Utf8PathBuf}; +use certificate::CertificateError; +use certificate::PemCertificate; +use std::borrow::Cow; +use tedge_config_macros::*; + +#[derive(thiserror::Error, Debug)] +pub enum ReadError { + #[error(transparent)] + ConfigNotSet(#[from] ConfigNotSet), + + #[error(transparent)] + Multi(#[from] MultiError), + + #[error("Config value {key}, cannot be read: {message} ")] + ReadOnlyNotFound { + key: Cow<'static, str>, + message: &'static str, + }, + + #[error("Derivation for `{key}` failed: {cause}")] + DerivationFailed { + key: Cow<'static, str>, + cause: String, + }, +} + +pub trait AppendRemoveItem { + type Item; + + fn append(current_value: Option, new_value: Self::Item) -> Option; + + fn remove(current_value: Option, remove_value: Self::Item) -> Option; +} + +impl AppendRemoveItem for T { + type Item = T; + + fn append(_current_value: Option, _new_value: Self::Item) -> Option { + unimplemented!() + } + + fn remove(_current_value: Option, _remove_value: Self::Item) -> Option { + unimplemented!() + } +} + +define_tedge_config! { + device: { + #[tedge_config(reader(function = "device_id"))] + #[doku(as = "String")] + id: Result, + + #[doku(as = "String")] + cert_path: Utf8PathBuf, + }, + + #[tedge_config(multi)] + c8y: { + device: { + #[tedge_config(reader(function = "c8y_device_id"))] + #[tedge_config(default(from_optional_key = "device.id"))] + #[doku(as = "String")] + id: Result, + + #[doku(as = "String")] + #[tedge_config(default(from_optional_key = "device.cert_path"))] + cert_path: Utf8PathBuf, + } + }, +} + +fn device_id( + device: &TEdgeConfigReaderDevice, + dto_value: &OptionalConfig, +) -> Result { + match dto_value.or_none() { + Some(dto_value) => Ok(dto_value.to_owned()), + None => { + let cert = device.cert_path.or_config_not_set()?; + device_id_from_cert(cert) + } + } +} + +fn c8y_device_id( + c8y_device: &TEdgeConfigReaderC8yDevice, + dto_value: &OptionalConfig, +) -> Result { + match dto_value.or_none() { + Some(dto_value) => Ok(dto_value.to_owned()), + None => { + let cert = c8y_device.cert_path.or_config_not_set()?; + device_id_from_cert(cert) + } + } +} + +fn device_id_from_cert(cert_path: &Utf8Path) -> Result { + let pem = PemCertificate::from_pem_file(cert_path) + .map_err(|err| cert_error_into_config_error(ReadableKey::DeviceId.to_cow_str(), err))?; + let device_id = pem + .subject_common_name() + .map_err(|err| cert_error_into_config_error(ReadableKey::DeviceId.to_cow_str(), err))?; + Ok(device_id) +} + +fn cert_error_into_config_error(key: Cow<'static, str>, err: CertificateError) -> ReadError { + match &err { + CertificateError::IoError { error, .. } => match error.kind() { + std::io::ErrorKind::NotFound => ReadError::ReadOnlyNotFound { + key, + message: concat!( + "The device id is read from the device certificate.\n", + "To set 'device.id' to some , you can use `tedge cert create --device-id `.", + ), + }, + _ => ReadError::DerivationFailed { + key, + cause: format!("{}", err), + }, + }, + _ => ReadError::DerivationFailed { + key, + cause: format!("{}", err), + }, + } +} + +fn read_config(toml: &str) -> TEdgeConfigReader { + let c8y_dto = toml::from_str(toml).unwrap(); + TEdgeConfigReader::from_dto(&c8y_dto, &TEdgeConfigLocation) +} + +fn main() { + let config = read_config("device.id = \"test-device-id\""); + let c8y = config.c8y.try_get::<&str>(None).unwrap(); + + assert_eq!(config.device.id().unwrap(), "test-device-id"); + assert_eq!(c8y.device.id().unwrap(), "test-device-id"); + + let config = read_config("device.id = \"test-device-id\"\nc8y.device.id = \"c8y-device-id\""); + let c8y = config.c8y.try_get::<&str>(None).unwrap(); + + assert_eq!(config.device.id().unwrap(), "test-device-id"); + assert_eq!(c8y.device.id().unwrap(), "c8y-device-id"); +} diff --git a/crates/common/tedge_config_macros/impl/src/reader.rs b/crates/common/tedge_config_macros/impl/src/reader.rs index 894f9d19036..6e24a1ec4a6 100644 --- a/crates/common/tedge_config_macros/impl/src/reader.rs +++ b/crates/common/tedge_config_macros/impl/src/reader.rs @@ -152,9 +152,9 @@ fn generate_structs( } let lazy_reader_impls = lazy_readers.iter().map( - |(name, ty, function, parent_ty, id, _dto_ty)| -> syn::ItemImpl { + |(_, ty, function, parent_ty, id, _dto_ty)| -> syn::ItemImpl { if let Some((ok, err)) = extract_type_from_result(ty) { - parse_quote_spanned! {name.span()=> + parse_quote_spanned! {function.span()=> impl #parent_ty { pub fn #id(&self) -> Result<&#ok, #err> { self.#id.0.get_or_try_init(|| #function(self, &self.#id.1)) @@ -162,7 +162,7 @@ fn generate_structs( } } } else { - parse_quote_spanned! {name.span()=> + parse_quote_spanned! {function.span()=> impl #parent_ty { pub fn #id(&self) -> &#ty { self.#id.0.get_or_init(|| #function(self, &self.#id.1)) @@ -372,18 +372,20 @@ fn reader_value_for_field<'a>( observed_keys, )?; - let (default, value) = if matches!( - &rw_field.default, - FieldDefault::FromOptionalKey(_) - ) { + let (default, value) = if rw_field.reader.function.is_some() { + ( + quote_spanned!(default_key.span()=> #default.1.into()), + quote_spanned!(rw_field.ident.span()=> OptionalConfig::Present { value: value.clone(), key: #key }), + ) + } else if matches!(&rw_field.default, FieldDefault::FromOptionalKey(_)) { ( quote_spanned!(default_key.span()=> #default.map(|v| v.into())), - quote_spanned!(default_key.span()=> OptionalConfig::Present { value: value.clone(), key: #key }), + quote_spanned!(rw_field.ident.span()=> OptionalConfig::Present { value: value.clone(), key: #key }), ) } else { ( quote_spanned!(default_key.span()=> #default.into()), - quote_spanned!(default_key.span()=> value.clone()), + quote_spanned!(rw_field.ident.span()=> value.clone()), ) }; @@ -541,7 +543,7 @@ fn generate_conversions( FieldOrGroup::Field(field) => { let name = field.ident(); let value = reader_value_for_field(field, &parents, root_fields, Vec::new())?; - field_conversions.push(quote!(#name: #value)); + field_conversions.push(quote_spanned!(name.span()=> #name: #value)); } FieldOrGroup::Group(group) if !group.reader.skip => { let sub_reader_name = prefixed_type_name(name, group); @@ -561,7 +563,7 @@ fn generate_conversions( }) .collect(); field_conversions.push( - quote!(#name: #sub_reader_name::from_dto(dto, location, #(#extra_call_args),*)), + quote_spanned!(name.span()=> #name: #sub_reader_name::from_dto(dto, location, #(#extra_call_args),*)), ); let sub_conversions = generate_conversions(&sub_reader_name, &group.contents, parents, root_fields)?; @@ -597,7 +599,7 @@ fn generate_conversions( .intersperse(".".to_owned()) .collect::(); let new_arg2 = extra_call_args.last().unwrap().clone(); - field_conversions.push(quote!(#name: dto.#(#read_path).*.map_keys(|#new_arg2| #sub_reader_name::from_dto(dto, location, #(#extra_call_args),*), #parent_key))); + field_conversions.push(quote_spanned!(name.span()=> #name: dto.#(#read_path).*.map_keys(|#new_arg2| #sub_reader_name::from_dto(dto, location, #(#extra_call_args),*), #parent_key))); parents.push(new_arg); let sub_conversions = generate_conversions(&sub_reader_name, &group.contents, parents, root_fields)?; @@ -609,7 +611,7 @@ fn generate_conversions( } } - Ok(quote! { + Ok(quote_spanned! {name.span()=> impl #name { #[allow(unused, clippy::clone_on_copy, clippy::useless_conversion)] #[automatically_derived] diff --git a/crates/core/tedge/src/cli/upload/mod.rs b/crates/core/tedge/src/cli/upload/mod.rs index fcf8fa916cf..080f8401ee9 100644 --- a/crates/core/tedge/src/cli/upload/mod.rs +++ b/crates/core/tedge/src/cli/upload/mod.rs @@ -79,8 +79,9 @@ impl BuildCommand for UploadCmd { let identity = config.http.client.auth.identity()?; let cloud_root_certs = config.cloud_root_certs(); let c8y = C8yEndPoint::local_proxy(&config, profile.as_deref())?; + let c8y_config = config.c8y.try_get(profile.as_deref())?; let device_id = match device_id { - None => config.device.id()?.clone(), + None => c8y_config.device.id()?.clone(), Some(device_id) => device_id, }; let text = text.unwrap_or_else(|| format!("Uploaded file: {file:?}")); From 857524a56d233941cc14e9961bbbcea64099d828 Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Fri, 10 Jan 2025 10:06:46 +0000 Subject: [PATCH 3/3] Make device.id field private --- .../src/tedge_config_cli/tedge_config.rs | 42 ++++++++------- .../examples/reader_function.rs | 3 +- .../tedge_config_macros/impl/src/reader.rs | 53 +++++++++++-------- crates/core/tedge/tests/main.rs | 48 ----------------- 4 files changed, 57 insertions(+), 89 deletions(-) diff --git a/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs b/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs index 106eff169cd..ecc601fe641 100644 --- a/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs +++ b/crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs @@ -428,7 +428,7 @@ define_tedge_config! { device: { /// Identifier of the device within the fleet. It must be globally /// unique and is derived from the device certificate. - #[tedge_config(reader(function = "device_id"))] + #[tedge_config(reader(function = "device_id", private))] #[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")] #[tedge_config(note = "This setting is derived from the device certificate and is therefore read only.")] #[doku(as = "String")] @@ -674,12 +674,8 @@ define_tedge_config! { device: { /// Identifier of the device within the fleet. It must be globally /// unique and is derived from the device certificate. - #[tedge_config(readonly( - write_error = "\ - The device id is read from the device certificate and cannot be set directly.\n\ - To set 'device.id' to some , you can use `tedge cert create --device-id `.", - function = "az_device_id", - ))] + #[tedge_config(reader(function = "az_device_id"))] + #[tedge_config(default(from_optional_key = "device.id"))] #[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")] #[tedge_config(note = "This setting is derived from the device certificate and is therefore read only.")] #[doku(as = "String")] @@ -744,12 +740,8 @@ define_tedge_config! { device: { /// Identifier of the device within the fleet. It must be globally /// unique and is derived from the device certificate. - #[tedge_config(readonly( - write_error = "\ - The device id is read from the device certificate and cannot be set directly.\n\ - To set 'device.id' to some , you can use `tedge cert create --device-id `.", - function = "aws_device_id", - ))] + #[tedge_config(reader(function = "aws_device_id"))] + #[tedge_config(default(from_optional_key = "device.id"))] #[tedge_config(example = "Raspberrypi-4d18303a-6d3a-11eb-b1a6-175f6bb72665")] #[tedge_config(note = "This setting is derived from the device certificate and is therefore read only.")] #[doku(as = "String")] @@ -1357,7 +1349,7 @@ fn device_id( ) -> Result { match dto_value.or_none() { Some(id) => Ok(id.clone()), - None => device_id_from_cert(&device.cert_path) + None => device_id_from_cert(&device.cert_path), } } @@ -1367,16 +1359,28 @@ fn c8y_device_id( ) -> Result { match dto_value.or_none() { Some(id) => Ok(id.clone()), - None => device_id_from_cert(&c8y_device.cert_path) + None => device_id_from_cert(&c8y_device.cert_path), } } -fn az_device_id(device: &TEdgeConfigReaderAzDevice, _: &()) -> Result { - device_id_from_cert(&device.cert_path) +fn az_device_id( + az_device: &TEdgeConfigReaderAzDevice, + dto_value: &OptionalConfig, +) -> Result { + match dto_value.or_none() { + Some(id) => Ok(id.clone()), + None => device_id_from_cert(&az_device.cert_path), + } } -fn aws_device_id(device: &TEdgeConfigReaderAwsDevice, _: &()) -> Result { - device_id_from_cert(&device.cert_path) +fn aws_device_id( + aws_device: &TEdgeConfigReaderAwsDevice, + dto_value: &OptionalConfig, +) -> Result { + match dto_value.or_none() { + Some(id) => Ok(id.clone()), + None => device_id_from_cert(&aws_device.cert_path), + } } fn cert_error_into_config_error(key: Cow<'static, str>, err: CertificateError) -> ReadError { diff --git a/crates/common/tedge_config_macros/examples/reader_function.rs b/crates/common/tedge_config_macros/examples/reader_function.rs index 90d5ca8f2cc..3d15e8618f9 100644 --- a/crates/common/tedge_config_macros/examples/reader_function.rs +++ b/crates/common/tedge_config_macros/examples/reader_function.rs @@ -1,4 +1,5 @@ -use camino::{Utf8Path, Utf8PathBuf}; +use camino::Utf8Path; +use camino::Utf8PathBuf; use certificate::CertificateError; use certificate::PemCertificate; use std::borrow::Cow; diff --git a/crates/common/tedge_config_macros/impl/src/reader.rs b/crates/common/tedge_config_macros/impl/src/reader.rs index 6e24a1ec4a6..820c166b86c 100644 --- a/crates/common/tedge_config_macros/impl/src/reader.rs +++ b/crates/common/tedge_config_macros/impl/src/reader.rs @@ -57,24 +57,25 @@ fn generate_structs( let ty = field.ty(); attrs.push(field.attrs().to_vec()); idents.push(field.ident()); - if let Some((function, field)) = field.reader_function() { - let name = field.lazy_reader_name(&parents); - let parent_ty = field.parent_name(&parents); + if let Some((function, rw_field)) = field.reader_function() { + let name = rw_field.lazy_reader_name(&parents); + let parent_ty = rw_field.parent_name(&parents); tys.push(parse_quote_spanned!(ty.span()=> #name)); - let dto_ty: syn::Type = match extract_type_from_result(&field.ty) { + let dto_ty: syn::Type = match extract_type_from_result(&rw_field.ty) { Some((ok, _err)) => parse_quote!(OptionalConfig<#ok>), None => { - let ty = &field.ty; + let ty = &rw_field.ty; parse_quote!(OptionalConfig<#ty>) } }; lazy_readers.push(( name, - &field.ty, + &rw_field.ty, function, parent_ty, - field.ident.clone(), + rw_field.ident.clone(), dto_ty.clone(), + visibility(field), )); vis.push(parse_quote!()); } else if field.is_optional() { @@ -83,17 +84,18 @@ fn generate_structs( true => parse_quote!(), false => parse_quote!(pub), }); - } else if let Some(field) = field.read_only() { - let name = field.lazy_reader_name(&parents); - let parent_ty = field.parent_name(&parents); - tys.push(parse_quote_spanned!(field.ty.span()=> #name)); + } else if let Some(ro_field) = field.read_only() { + let name = ro_field.lazy_reader_name(&parents); + let parent_ty = ro_field.parent_name(&parents); + tys.push(parse_quote_spanned!(ro_field.ty.span()=> #name)); lazy_readers.push(( name, - &field.ty, - &field.readonly.function, + &ro_field.ty, + &ro_field.readonly.function, parent_ty, - field.ident.clone(), + ro_field.ident.clone(), parse_quote!(()), + visibility(field), )); vis.push(parse_quote!()); } else { @@ -152,11 +154,11 @@ fn generate_structs( } let lazy_reader_impls = lazy_readers.iter().map( - |(_, ty, function, parent_ty, id, _dto_ty)| -> syn::ItemImpl { + |(_, ty, function, parent_ty, id, _dto_ty, vis)| -> syn::ItemImpl { if let Some((ok, err)) = extract_type_from_result(ty) { parse_quote_spanned! {function.span()=> impl #parent_ty { - pub fn #id(&self) -> Result<&#ok, #err> { + #vis fn #id(&self) -> Result<&#ok, #err> { self.#id.0.get_or_try_init(|| #function(self, &self.#id.1)) } } @@ -164,7 +166,7 @@ fn generate_structs( } else { parse_quote_spanned! {function.span()=> impl #parent_ty { - pub fn #id(&self) -> &#ty { + #vis fn #id(&self) -> &#ty { self.#id.0.get_or_init(|| #function(self, &self.#id.1)) } } @@ -176,7 +178,7 @@ fn generate_structs( let (lr_names, lr_tys, lr_dto_tys): (Vec<_>, Vec<_>, Vec<_>) = lazy_readers .iter() .map( - |(name, ty, _, _, _, dto_ty)| match extract_type_from_result(ty) { + |(name, ty, _, _, _, dto_ty, _)| match extract_type_from_result(ty) { Some((ok, _err)) => (name, ok, dto_ty), None => (name, *ty, dto_ty), }, @@ -212,6 +214,14 @@ fn generate_structs( }) } +fn visibility(field: &ConfigurableField) -> syn::Visibility { + if field.reader().private { + parse_quote!() + } else { + parse_quote!(pub) + } +} + fn find_field<'a>( mut fields: &'a [FieldOrGroup], key: &Punctuated, @@ -827,7 +837,7 @@ mod tests { fn generate_structs_generates_getter_for_reader_function_value() { let input: crate::input::Configuration = parse_quote!( device: { - #[tedge_config(reader(function = "device_id"))] + #[tedge_config(reader(function = "device_id", private))] id: String, }, ); @@ -845,7 +855,7 @@ mod tests { let expected = parse_quote! { impl TEdgeConfigReaderDevice { - pub fn id(&self) -> &String { + fn id(&self) -> &String { self.id.0.get_or_init(|| device_id(self, &self.id.1)) } } @@ -1030,7 +1040,8 @@ mod tests { } }, ) - .map(|v| v.into()) + .1 + .into() } }, ), diff --git a/crates/core/tedge/tests/main.rs b/crates/core/tedge/tests/main.rs index 04773a3c8ed..f98bc425ea1 100644 --- a/crates/core/tedge/tests/main.rs +++ b/crates/core/tedge/tests/main.rs @@ -133,54 +133,6 @@ mod tests { Ok(()) } - #[test] - fn run_config_set_get_unset_read_only_key() -> Result<(), Box> { - let temp_dir = tempfile::tempdir().unwrap(); - let temp_dir_path = temp_dir.path(); - let test_home_str = temp_dir_path.to_str().unwrap(); - - let device_id = "test"; - - // allowed to get read-only key by CLI - let mut get_device_id_cmd = tedge_command_with_test_home([ - "--config-dir", - test_home_str, - "config", - "get", - "device.id", - ])?; - - get_device_id_cmd - .assert() - .failure() - .stderr(predicate::str::contains("device.id")); - - // forbidden to set read-only key by CLI - let mut set_device_id_cmd = tedge_command_with_test_home([ - "--config-dir", - test_home_str, - "config", - "set", - "device.id", - device_id, - ])?; - - set_device_id_cmd.assert().failure(); - - // forbidden to unset read-only key by CLI - let mut unset_device_id_cmd = tedge_command_with_test_home([ - "--config-dir", - test_home_str, - "config", - "unset", - "device.id", - ])?; - - unset_device_id_cmd.assert().failure(); - - Ok(()) - } - // #[test_case(config key, config value, expected unset value)] #[test_case( "c8y.url",