diff --git a/proxmox-api-macro/src/api/enums.rs b/proxmox-api-macro/src/api/enums.rs index 344ffa39..3c35bc09 100644 --- a/proxmox-api-macro/src/api/enums.rs +++ b/proxmox-api-macro/src/api/enums.rs @@ -85,10 +85,8 @@ pub fn handle_enum( .schema(); } - #[automatically_derived] - impl ::proxmox::api::schema::Updatable for #name { + impl ::proxmox::api::schema::UpdaterType for #name { type Updater = Option; - const UPDATER_IS_OPTION: bool = true; } }) } diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs index 5a644b35..26b8e437 100644 --- a/proxmox-api-macro/src/api/mod.rs +++ b/proxmox-api-macro/src/api/mod.rs @@ -473,9 +473,7 @@ impl ToTokens for OptionType { fn to_tokens(&self, tokens: &mut TokenStream) { match self { OptionType::Bool(b) => b.to_tokens(tokens), - OptionType::Updater(ty) => tokens.extend(quote! { - <#ty as ::proxmox::api::schema::Updatable>::UPDATER_IS_OPTION - }), + OptionType::Updater(_) => tokens.extend(quote! { true }), } } } diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs index f59c3940..4bc6b759 100644 --- a/proxmox-api-macro/src/api/structs.rs +++ b/proxmox-api-macro/src/api/structs.rs @@ -62,9 +62,8 @@ fn handle_unit_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result - impl ::proxmox::api::schema::Updatable for #name { + impl ::proxmox::api::schema::UpdaterType for #name { type Updater = Option; - const UPDATER_IS_OPTION: bool = true; } }); @@ -404,6 +403,7 @@ fn derive_updater( mut schema: Schema, original_struct: &mut syn::ItemStruct, ) -> Result { + let original_name = &original_struct.ident; stru.ident = Ident::new(&format!("{}Updater", stru.ident), stru.ident.span()); if !util::derived_items(&original_struct.attrs).any(|p| p.is_ident("Default")) { @@ -413,19 +413,7 @@ fn derive_updater( )); } - original_struct.attrs.push(util::make_derive_attribute( - Span::call_site(), - quote::quote! { ::proxmox::api::schema::Updatable }, - )); - let updater_name = &stru.ident; - let updater_name_str = syn::LitStr::new(&updater_name.to_string(), updater_name.span()); - original_struct.attrs.push(util::make_attribute( - Span::call_site(), - util::make_path(Span::call_site(), false, &["updatable"]), - quote::quote! { (updater = #updater_name_str) }, - )); - let mut all_of_schemas = TokenStream::new(); let mut is_empty_impl = TokenStream::new(); @@ -454,16 +442,22 @@ fn derive_updater( }; if !is_empty_impl.is_empty() { - output = quote::quote!( - #output + output.extend(quote::quote!( + #[automatically_derived] impl ::proxmox::api::schema::Updater for #updater_name { fn is_empty(&self) -> bool { #is_empty_impl } } - ); + )); } + output.extend(quote::quote!( + impl ::proxmox::api::schema::UpdaterType for #original_name { + type Updater = #updater_name; + } + )); + Ok(output) } @@ -518,7 +512,7 @@ fn handle_updater_field( path: util::make_path( span, true, - &["proxmox", "api", "schema", "Updatable", "Updater"], + &["proxmox", "api", "schema", "UpdaterType", "Updater"], ), }; @@ -535,7 +529,8 @@ fn handle_updater_field( if field_schema.flatten_in_struct { let updater_ty = &field.ty; - all_of_schemas.extend(quote::quote! {&<#updater_ty as ::proxmox::api::schema::ApiType>::API_SCHEMA,}); + all_of_schemas + .extend(quote::quote! {&<#updater_ty as ::proxmox::api::schema::ApiType>::API_SCHEMA,}); } if !is_empty_impl.is_empty() { diff --git a/proxmox-api-macro/src/lib.rs b/proxmox-api-macro/src/lib.rs index 2db11379..9c35eeff 100644 --- a/proxmox-api-macro/src/lib.rs +++ b/proxmox-api-macro/src/lib.rs @@ -231,7 +231,7 @@ fn router_do(item: TokenStream) -> Result { # Deriving an `Updater`: - An "Updater" struct can be generated automatically for a type. This affects the `Updatable` + An "Updater" struct can be generated automatically for a type. This affects the `UpdaterType` trait implementation generated, as it will set the associated `type Updater = TheDerivedUpdater`. @@ -265,10 +265,10 @@ fn router_do(item: TokenStream) -> Result { #[api] /// An example of a simple struct type. pub struct MyTypeUpdater { - one: Option, // really ::Updater + one: Option, // really ::Updater #[serde(skip_serializing_if = "Option::is_none")] - opt: Option, // really as Updatable>::Updater + opt: Option, // really as UpdaterType>::Updater } impl Updater for MyTypeUpdater { @@ -277,36 +277,8 @@ fn router_do(item: TokenStream) -> Result { } } - impl Updatable for MyType { + impl UpdaterType for MyType { type Updater = MyTypeUpdater; - - fn update_from(&mut self, from: MyTypeUpdater, delete: &[T]) -> Result<(), Error> - where - T: AsRef, - { - for delete in delete { - match delete.as_ref() { - "opt" => { self.opt = None; } - _ => (), - } - } - - self.one.update_from(from.one)?; - self.opt.update_from(from.opt)?; - - Ok(()) - } - - fn try_build_from(from: MyTypeUpdater) -> Result { - Ok(Self { - // This amounts to `from.one.ok_or_else("cannot build from None value")?` - one: Updatable::try_build_from(from.one) - .map_err(|err| format_err!("failed to build value for field 'one': {}", err))?, - // This amounts to `from.opt` - opt: Updatable::try_build_from(from.opt) - .map_err(|err| format_err!("failed to build value for field 'opt': {}", err))?, - }) - } } ``` @@ -320,17 +292,17 @@ pub fn api(attr: TokenStream_1, item: TokenStream_1) -> TokenStream_1 { /// This is a dummy derive macro actually handled by `#[api]`! #[doc(hidden)] -#[proc_macro_derive(Updater, attributes(updater, updatable, serde))] +#[proc_macro_derive(Updater, attributes(updater, serde))] pub fn derive_updater(_item: TokenStream_1) -> TokenStream_1 { TokenStream_1::new() } -/// Create the default `Updatable` implementation from an `Option`. -#[proc_macro_derive(Updatable, attributes(updatable, serde))] -pub fn derive_updatable(item: TokenStream_1) -> TokenStream_1 { +/// Create the default `UpdaterType` implementation as an `Option`. +#[proc_macro_derive(UpdaterType, attributes(updater_type, serde))] +pub fn derive_updater_type(item: TokenStream_1) -> TokenStream_1 { let _error_guard = init_local_error(); let item: TokenStream = item.into(); - handle_error(item.clone(), updater::updatable(item).map_err(Error::from)).into() + handle_error(item.clone(), updater::updater_type(item).map_err(Error::from)).into() } thread_local!(static NON_FATAL_ERRORS: RefCell> = RefCell::new(None)); diff --git a/proxmox-api-macro/src/updater.rs b/proxmox-api-macro/src/updater.rs index b596f13c..aad0e95c 100644 --- a/proxmox-api-macro/src/updater.rs +++ b/proxmox-api-macro/src/updater.rs @@ -1,162 +1,43 @@ use proc_macro2::{Ident, Span, TokenStream}; -use quote::{quote, quote_spanned}; +use quote::quote_spanned; use syn::spanned::Spanned; -pub(crate) fn updatable(item: TokenStream) -> Result { +pub(crate) fn updater_type(item: TokenStream) -> Result { let item: syn::Item = syn::parse2(item)?; let full_span = item.span(); - match item { + Ok(match item { syn::Item::Struct(syn::ItemStruct { - fields: syn::Fields::Named(named), - attrs, ident, generics, .. - }) => derive_named_struct_updatable(attrs, full_span, ident, generics, named), - syn::Item::Struct(syn::ItemStruct { - attrs, - ident, - generics, - .. - }) => derive_default_updatable(attrs, full_span, ident, generics), + }) => derive_updater_type(full_span, ident, generics), syn::Item::Enum(syn::ItemEnum { - attrs, ident, generics, .. - }) => derive_default_updatable(attrs, full_span, ident, generics), - _ => bail!(item => "`Updatable` can only be derived for structs"), - } + }) => derive_updater_type(full_span, ident, generics), + _ => bail!(item => "`UpdaterType` cannot be derived for this type"), + }) } fn no_generics(generics: syn::Generics) { if let Some(lt) = generics.lt_token { - error!(lt => "deriving `Updatable` for a generic enum is not supported"); + error!(lt => "deriving `UpdaterType` for a generic enum is not supported"); } else if let Some(wh) = generics.where_clause { - error!(wh => "deriving `Updatable` on enums with generic bounds is not supported"); + error!(wh => "deriving `UpdaterType` on enums with generic bounds is not supported"); } } -fn derive_default_updatable( - attrs: Vec, +fn derive_updater_type( full_span: Span, ident: Ident, generics: syn::Generics, -) -> Result { +) -> TokenStream { no_generics(generics); - let args = UpdatableArgs::from_attributes(attrs); - if let Some(updater) = args.updater { - error!(updater => "`updater` updater attribute not supported for this type"); - } - - Ok(default_updatable(full_span, ident)) -} - -fn default_updatable(full_span: Span, ident: Ident) -> TokenStream { quote_spanned! { full_span => - #[automatically_derived] - impl ::proxmox::api::schema::Updatable for #ident { - type Updater = Option<#ident>; - const UPDATER_IS_OPTION: bool = true; + impl ::proxmox::api::schema::UpdaterType for #ident { + type Updater = Option; } } } - -fn derive_named_struct_updatable( - attrs: Vec, - full_span: Span, - ident: Ident, - generics: syn::Generics, - _fields: syn::FieldsNamed, -) -> Result { - no_generics(generics); - - let args = UpdatableArgs::from_attributes(attrs); - let updater = match args.updater { - Some(updater) => updater, - None => return Ok(default_updatable(full_span, ident)), - }; - - Ok(quote! { - #[automatically_derived] - impl ::proxmox::api::schema::Updatable for #ident { - type Updater = #updater; - const UPDATER_IS_OPTION: bool = false; - } - }) -} - -#[derive(Default)] -struct UpdatableArgs { - updater: Option, -} - -impl UpdatableArgs { - fn from_attributes(attributes: Vec) -> Self { - let mut this = Self::default(); - - for_attributes(attributes, "updatable", |meta| this.parse_nested(meta)); - - this - } - - fn parse_nested(&mut self, meta: syn::NestedMeta) -> Result<(), syn::Error> { - match meta { - syn::NestedMeta::Meta(syn::Meta::NameValue(nv)) => self.parse_name_value(nv), - other => bail!(other => "invalid updater argument"), - } - } - - fn parse_name_value(&mut self, nv: syn::MetaNameValue) -> Result<(), syn::Error> { - if nv.path.is_ident("updater") { - let updater: syn::Type = match nv.lit { - // we could use `s.parse()` but it doesn't make sense to put the original struct - // name as spanning info here, so instead, we use the call site: - syn::Lit::Str(s) => syn::parse_str(&s.value())?, - other => bail!(other => "updater argument must be a string literal"), - }; - - if self.updater.is_some() { - error!(updater.span(), "multiple 'updater' attributes not allowed"); - } - - self.updater = Some(updater); - Ok(()) - } else { - bail!(nv.path => "unrecognized updater argument"); - } - } -} - -/// Non-fatally go through all `updater` attributes. -fn for_attributes(attributes: Vec, attr_name: &str, mut func: F) -where - F: FnMut(syn::NestedMeta) -> Result<(), syn::Error>, -{ - for meta in meta_iter(attributes) { - let list = match meta { - syn::Meta::List(list) if list.path.is_ident(attr_name) => list, - _ => continue, - }; - - for entry in list.nested { - match func(entry) { - Ok(()) => (), - Err(err) => crate::add_error(err), - } - } - } -} - -fn meta_iter( - attributes: impl IntoIterator, -) -> impl Iterator { - attributes.into_iter().filter_map(|attr| { - if attr.style != syn::AttrStyle::Outer { - return None; - } - - attr.parse_meta().ok() - }) -} diff --git a/proxmox-api-macro/tests/allof.rs b/proxmox-api-macro/tests/allof.rs index 9fcf9795..f14f6a1e 100644 --- a/proxmox-api-macro/tests/allof.rs +++ b/proxmox-api-macro/tests/allof.rs @@ -4,7 +4,7 @@ use anyhow::Error; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; -use proxmox::api::schema; +use proxmox::api::schema::{self, ApiType}; use proxmox_api_macro::api; pub const NAME_SCHEMA: schema::Schema = schema::StringSchema::new("Name.").schema(); diff --git a/proxmox-api-macro/tests/int-limits.rs b/proxmox-api-macro/tests/int-limits.rs index 7606a14c..d8a47391 100644 --- a/proxmox-api-macro/tests/int-limits.rs +++ b/proxmox-api-macro/tests/int-limits.rs @@ -1,5 +1,6 @@ //! Test the automatic addition of integer limits. +use proxmox::api::schema::ApiType; use proxmox_api_macro::api; /// An i16: -32768 to 32767. diff --git a/proxmox-api-macro/tests/types.rs b/proxmox-api-macro/tests/types.rs index 80d09ba9..2fd83808 100644 --- a/proxmox-api-macro/tests/types.rs +++ b/proxmox-api-macro/tests/types.rs @@ -3,7 +3,7 @@ #![allow(dead_code)] -use proxmox::api::schema::{self, EnumEntry}; +use proxmox::api::schema::{self, ApiType, EnumEntry}; use proxmox_api_macro::api; use anyhow::Error; diff --git a/proxmox-api-macro/tests/updater.rs b/proxmox-api-macro/tests/updater.rs index 0ccde105..e0662418 100644 --- a/proxmox-api-macro/tests/updater.rs +++ b/proxmox-api-macro/tests/updater.rs @@ -1,16 +1,31 @@ +#![allow(dead_code)] + use proxmox::api::api; -use proxmox::api::schema::{Schema, StringSchema, Updatable, Updater}; +use proxmox::api::schema::{ApiType, Updater, UpdaterType}; -#[derive(Updatable)] -pub struct Custom(String); - -impl proxmox::api::schema::ApiType for Custom { - const API_SCHEMA: Schema = StringSchema::new("Custom String") - .min_length(3) - .max_length(64) - .schema(); +// Helpers for type checks: +struct AssertTypeEq(T); +macro_rules! assert_type_eq { + ($what:ident, $a:ty, $b:ty) => { + #[allow(dead_code, unreachable_patterns)] + fn $what(have: AssertTypeEq<$a>) { + match have { + AssertTypeEq::<$b>(_) => (), + } + } + }; } +#[api(min_length: 3, max_length: 64)] +#[derive(UpdaterType)] +/// Custom String. +pub struct Custom(String); +assert_type_eq!( + custom_type, + ::Updater, + Option +); + #[api] /// An example of a simple struct type. #[derive(Updater)] @@ -19,11 +34,34 @@ pub struct Simple { /// A test string. one_field: String, - /// An optional auto-derived value for testing: + /// Another test value. #[serde(skip_serializing_if = "Option::is_empty")] opt: Option, } +#[test] +fn test_simple() { + pub const TEST_SCHEMA: ::proxmox::api::schema::Schema = + ::proxmox::api::schema::ObjectSchema::new( + "An example of a simple struct type.", + &[ + ( + "one-field", + true, + &::proxmox::api::schema::StringSchema::new("A test string.").schema(), + ), + ( + "opt", + true, + &::proxmox::api::schema::StringSchema::new("Another test value.").schema(), + ), + ], + ) + .schema(); + + assert_eq!(TEST_SCHEMA, SimpleUpdater::API_SCHEMA); +} + #[api( properties: { simple: { type: Simple }, @@ -51,11 +89,10 @@ pub struct Complex { #[derive(Updater)] #[serde(rename_all = "kebab-case")] pub struct SuperComplex { - /// An extra field not part of the flattened struct. + /// An extra field. extra: String, - #[serde(skip_serializing_if = "Updater::is_empty")] - simple: Option, + simple: Simple, /// A field which should not appear in the updater. #[updater(skip)] @@ -64,3 +101,27 @@ pub struct SuperComplex { /// A custom type with an Updatable implementation. custom: Custom, } +#[test] +fn test_super_complex() { + pub const TEST_SCHEMA: ::proxmox::api::schema::Schema = + ::proxmox::api::schema::ObjectSchema::new( + "One of the baaaad cases.", + &[ + ("custom", true, & as ApiType>::API_SCHEMA), + ( + "extra", + true, + &::proxmox::api::schema::StringSchema::new("An extra field.").schema(), + ), + ( + "simple", + true, + //&<::Updater as ApiType>::API_SCHEMA, + &SimpleUpdater::API_SCHEMA, + ), + ], + ) + .schema(); + + assert_eq!(TEST_SCHEMA, SuperComplexUpdater::API_SCHEMA); +} diff --git a/proxmox/src/api/schema.rs b/proxmox/src/api/schema.rs index 751dc1d1..e3ee7a31 100644 --- a/proxmox/src/api/schema.rs +++ b/proxmox/src/api/schema.rs @@ -1525,39 +1525,34 @@ fn test_verify_complex_array() { /// API types are "updatable" in order to support derived "Updater" structs more easily. /// /// By default, any API type is "updatable" by an `Option`. For types which do not use the -/// `#[api]` macro, this will need to be explicitly created (or derived via `#[derive(Updatable)]`. -pub trait Updatable: Sized { +/// `#[api]` macro, this will need to be explicitly created (or derived via `#[derive(UpdaterType)]`. +pub trait UpdaterType: Sized { type Updater: Updater; - /// This should always be true for the "default" updaters which are just `Option` types. - /// Types which are not wrapped in `Option` must set this to `false`. - const UPDATER_IS_OPTION: bool; } #[cfg(feature = "api-macro")] -pub use proxmox_api_macro::Updatable; +pub use proxmox_api_macro::UpdaterType; #[cfg(feature = "api-macro")] #[doc(hidden)] pub use proxmox_api_macro::Updater; -macro_rules! basic_updatable { +macro_rules! basic_updater_type { ($($ty:ty)*) => { $( - impl Updatable for $ty { + impl UpdaterType for $ty { type Updater = Option; - const UPDATER_IS_OPTION: bool = true; } )* }; } -basic_updatable! { bool u8 u16 u32 u64 i8 i16 i32 i64 usize isize f32 f64 String char } +basic_updater_type! { bool u8 u16 u32 u64 i8 i16 i32 i64 usize isize f32 f64 String char } -impl Updatable for Option +impl UpdaterType for Option where - T: Updatable, + T: UpdaterType, { type Updater = T::Updater; - const UPDATER_IS_OPTION: bool = true; } pub trait ApiType {