From cc065c175dc35e00d2f7306e722c485465a835d8 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Tue, 2 Feb 2021 14:09:15 +0100 Subject: [PATCH] api-macro: introduce updaters Signed-off-by: Wolfgang Bumiller --- proxmox-api-macro/Cargo.toml | 5 + proxmox-api-macro/src/api/method.rs | 20 +- proxmox-api-macro/src/api/mod.rs | 185 +++++++++++---- proxmox-api-macro/src/api/structs.rs | 176 +++++++++++--- proxmox-api-macro/src/lib.rs | 15 ++ proxmox-api-macro/src/updater.rs | 286 +++++++++++++++++++++++ proxmox-api-macro/src/util.rs | 187 ++++++++++++++- proxmox-api-macro/tests/updater.rs | 330 +++++++++++++++++++++++++++ 8 files changed, 1107 insertions(+), 97 deletions(-) create mode 100644 proxmox-api-macro/src/updater.rs create mode 100644 proxmox-api-macro/tests/updater.rs diff --git a/proxmox-api-macro/Cargo.toml b/proxmox-api-macro/Cargo.toml index 07e9988a..7a999353 100644 --- a/proxmox-api-macro/Cargo.toml +++ b/proxmox-api-macro/Cargo.toml @@ -23,3 +23,8 @@ proxmox = { version = "0.10.0", path = "../proxmox", features = [ "test-harness" serde = "1.0" serde_derive = "1.0" serde_json = "1.0" + +# [features] +# # Used to quickly filter out the serde derive noise when using `cargo expand` for debugging! +# # Add this in case you need it, but don't commit it (to avoid debcargo picking this up)! +# noserde = [] diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs index a03e6542..04cfc33e 100644 --- a/proxmox-api-macro/src/api/method.rs +++ b/proxmox-api-macro/src/api/method.rs @@ -17,7 +17,7 @@ use syn::spanned::Spanned; use syn::visit_mut::{self, VisitMut}; use syn::Ident; -use super::{ObjectEntry, Schema, SchemaItem}; +use super::{ObjectEntry, Schema, SchemaItem, SchemaObject}; use crate::util::{self, FieldName, JSONObject, JSONValue, Maybe}; /// A return type in a schema can have an `optional` flag. Other than that it is just a regular @@ -90,7 +90,7 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result Schema { span: Span::call_site(), description: Maybe::None, - item: SchemaItem::Object(Default::default()), + item: SchemaItem::Object(SchemaObject::new(Span::call_site())), properties: Vec::new(), }, }; @@ -274,10 +274,10 @@ fn handle_function_signature( // try to infer the type in the schema if it is not specified explicitly: let is_option = util::infer_type(&mut entry.schema, &*pat_type.ty)?; let has_default = entry.schema.find_schema_property("default").is_some(); - if !is_option && entry.optional && !has_default { + if !is_option && entry.optional.expect_bool() && !has_default { error!(pat_type => "optional types need a default or be an Option"); } - if has_default && !entry.optional { + if has_default && !entry.optional.expect_bool() { error!(pat_type => "non-optional parameter cannot have a default"); } } else { @@ -555,7 +555,7 @@ fn extract_normal_parameter( .transpose()? }); - if !param.entry.optional { + if !param.entry.optional.expect_bool() { // Non-optional types need to be extracted out of the option though (unless // they have a default): // @@ -585,13 +585,13 @@ fn extract_normal_parameter( pub const #name: #ty = #def; }); - if param.entry.optional && no_option_type { + if param.entry.optional.expect_bool() && no_option_type { // Optional parameter without an Option type requires a default: body.extend(quote_spanned! { span => .unwrap_or(#name) }); } - } else if param.entry.optional && no_option_type { + } else if param.entry.optional.expect_bool() && no_option_type { // FIXME: we should not be able to reach this without having produced another // error above already anyway? error!(param.ty => "Optional parameter without Option requires a default"); @@ -684,10 +684,12 @@ fn serialize_input_schema( let mut all_of_schemas = TokenStream::new(); for entry in flattened { - if entry.optional { + if entry.optional.expect_bool() { + // openapi & json schema don't exactly have a proper way to represent + // this, so we simply refuse: error!( entry.schema.span, - "optional flattened parameters are not supported" + "optional flattened parameters are not supported (by JSONSchema)" ); } diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs index 5994f19f..5c7d912d 100644 --- a/proxmox-api-macro/src/api/mod.rs +++ b/proxmox-api-macro/src/api/mod.rs @@ -12,7 +12,7 @@ use std::convert::{TryFrom, TryInto}; use anyhow::Error; use proc_macro2::{Span, TokenStream}; -use quote::{quote, quote_spanned}; +use quote::{quote, quote_spanned, ToTokens}; use syn::parse::{Parse, ParseStream, Parser}; use syn::spanned::Spanned; use syn::{Expr, ExprPath, Ident}; @@ -66,6 +66,7 @@ pub const NUMBERNAMES: &[&str] = &["Number", "f32", "f64"]; /// ObjectSchema::new("text", &[ ... ]).foo(bar) /// } /// ``` +#[derive(Clone)] pub struct Schema { span: Span, @@ -137,7 +138,7 @@ impl Schema { Self { span, description: Maybe::None, - item: SchemaItem::Object(SchemaObject::new()), + item: SchemaItem::Object(SchemaObject::new(span)), properties: Vec::new(), } } @@ -216,12 +217,13 @@ impl Schema { } } +#[derive(Clone)] pub enum SchemaItem { - Null, - Boolean, - Integer, - Number, - String, + Null(Span), + Boolean(Span), + Integer(Span), + Number(Span), + String(Span), Object(SchemaObject), Array(SchemaArray), ExternType(ExprPath), @@ -230,6 +232,21 @@ pub enum SchemaItem { } impl SchemaItem { + pub fn span(&self) -> Span { + match self { + SchemaItem::Null(span) => *span, + SchemaItem::Boolean(span) => *span, + SchemaItem::Integer(span) => *span, + SchemaItem::Number(span) => *span, + SchemaItem::String(span) => *span, + SchemaItem::Object(inner) => inner.span, + SchemaItem::Array(inner) => inner.span, + SchemaItem::ExternType(inner) => inner.span(), + SchemaItem::ExternSchema(inner) => inner.span(), + SchemaItem::Inferred(span) => *span, + } + } + /// If there's a `type` specified, parse it as that type. Otherwise check for keys which /// uniqueply identify the type, such as "properties" for type `Object`. fn try_extract_from(obj: &mut JSONObject) -> Result { @@ -267,15 +284,15 @@ impl SchemaItem { .ident; if name == "Null" { - Ok(SchemaItem::Null) + Ok(SchemaItem::Null(ty.span())) } else if name == "Boolean" || name == "bool" { - Ok(SchemaItem::Boolean) + Ok(SchemaItem::Boolean(ty.span())) } else if INTTYPES.iter().any(|n| name == n.name) { - Ok(SchemaItem::Integer) + Ok(SchemaItem::Integer(ty.span())) } else if NUMBERNAMES.iter().any(|n| name == n) { - Ok(SchemaItem::Number) + Ok(SchemaItem::Number(ty.span())) } else if name == "String" { - Ok(SchemaItem::String) + Ok(SchemaItem::String(ty.span())) } else if name == "Object" { Ok(SchemaItem::Object(SchemaObject::try_extract_from(obj)?)) } else if name == "Array" { @@ -296,39 +313,49 @@ impl SchemaItem { move || description.ok_or_else(|| format_err!(span, "missing description")); match self { - SchemaItem::Null => { + SchemaItem::Null(span) => { let description = check_description()?; - ts.extend(quote! { ::proxmox::api::schema::NullSchema::new(#description) }); + ts.extend(quote_spanned! { *span => + ::proxmox::api::schema::NullSchema::new(#description) + }); } - SchemaItem::Boolean => { + SchemaItem::Boolean(span) => { let description = check_description()?; - ts.extend(quote! { ::proxmox::api::schema::BooleanSchema::new(#description) }); + ts.extend(quote_spanned! { *span => + ::proxmox::api::schema::BooleanSchema::new(#description) + }); } - SchemaItem::Integer => { + SchemaItem::Integer(span) => { let description = check_description()?; - ts.extend(quote! { ::proxmox::api::schema::IntegerSchema::new(#description) }); + ts.extend(quote_spanned! { *span => + ::proxmox::api::schema::IntegerSchema::new(#description) + }); } - SchemaItem::Number => { + SchemaItem::Number(span) => { let description = check_description()?; - ts.extend(quote! { ::proxmox::api::schema::NumberSchema::new(#description) }); + ts.extend(quote_spanned! { *span => + ::proxmox::api::schema::NumberSchema::new(#description) + }); } - SchemaItem::String => { + SchemaItem::String(span) => { let description = check_description()?; - ts.extend(quote! { ::proxmox::api::schema::StringSchema::new(#description) }); + ts.extend(quote_spanned! { *span => + ::proxmox::api::schema::StringSchema::new(#description) + }); } SchemaItem::Object(obj) => { let description = check_description()?; let mut elems = TokenStream::new(); obj.to_schema_inner(&mut elems)?; - ts.extend( - quote! { ::proxmox::api::schema::ObjectSchema::new(#description, &[#elems]) }, - ); + ts.extend(quote_spanned! { obj.span => + ::proxmox::api::schema::ObjectSchema::new(#description, &[#elems]) + }); } SchemaItem::Array(array) => { let description = check_description()?; let mut items = TokenStream::new(); array.to_schema(&mut items)?; - ts.extend(quote! { + ts.extend(quote_spanned! { array.span => ::proxmox::api::schema::ArraySchema::new(#description, &#items) }); } @@ -391,25 +418,84 @@ impl SchemaItem { } Ok(()) } + + pub fn check_object_mut(&mut self) -> Result<&mut SchemaObject, syn::Error> { + match self { + SchemaItem::Object(obj) => Ok(obj), + _ => bail!(self.span(), "expected object schema, found something else"), + } + } } +#[derive(Clone)] +pub enum OptionType { + /// All regular api types just have simple boolean expressions for whether the fields in an + /// object struct are optional. The only exception is updaters where this depends on the + /// updater type. + Bool(bool), + + /// An updater type uses its "base" type's field's updaters to determine whether the field is + /// supposed to be an option. + Updater(syn::Type), +} + +impl OptionType { + pub fn expect_bool(&self) -> bool { + match self { + OptionType::Bool(b) => *b, + _ => panic!( + "internal error: unexpected Updater dependent 'optional' value in macro context" + ), + } + } +} + +impl From for OptionType { + fn from(b: bool) -> Self { + OptionType::Bool(b) + } +} + +impl From for OptionType { + fn from(ty: syn::Type) -> Self { + OptionType::Updater(ty) + } +} + +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 + }), + } + } +} + +#[derive(Clone)] pub struct ObjectEntry { pub name: FieldName, - pub optional: bool, + pub optional: OptionType, pub schema: Schema, /// This is only valid for methods. Methods should reset this to false after dealing with it, /// as encountering this during schema serialization will always cause an error. pub flatten: Option, + + /// This is used for structs. We mark flattened fields because we need them to be "skipped" + /// when serializing inner the object schema. + pub flatten_in_struct: bool, } impl ObjectEntry { pub fn new(name: FieldName, optional: bool, schema: Schema) -> Self { Self { name, - optional, + optional: optional.into(), schema, flatten: None, + flatten_in_struct: false, } } @@ -419,24 +505,39 @@ impl ObjectEntry { } } -#[derive(Default)] +#[derive(Clone)] /// Contains a sorted list of properties: pub struct SchemaObject { + span: Span, properties_: Vec, } impl SchemaObject { - pub fn new() -> Self { + pub fn new(span: Span) -> Self { Self { + span, properties_: Vec::new(), } } + /// Check whether ther are any kind of fields defined in the struct, regardless of whether + /// they're flattened or not. #[inline] pub fn is_empty(&self) -> bool { self.properties_.is_empty() } + /// Check whether this object has any fields which aren't being flattened. + #[inline] + pub fn has_non_flattened_fields(&self) -> bool { + // be explicit about how to treat an empty list: + if self.properties_.is_empty() { + return false; + } + + self.properties_.iter().any(|prop| !prop.flatten_in_struct) + } + #[inline] fn properties_mut(&mut self) -> &mut [ObjectEntry] { &mut self.properties_ @@ -466,6 +567,7 @@ impl SchemaObject { fn try_extract_from(obj: &mut JSONObject) -> Result { let mut this = Self { + span: obj.span(), properties_: obj .remove_required_element("properties")? .into_object("object field definition")? @@ -509,6 +611,10 @@ impl SchemaObject { fn to_schema_inner(&self, ts: &mut TokenStream) -> Result<(), Error> { for element in self.properties_.iter() { + if element.flatten_in_struct { + continue; + } + if let Some(span) = element.flatten { error!( span, @@ -518,7 +624,7 @@ impl SchemaObject { } let key = element.name.as_str(); - let optional = element.optional; + let optional = &element.optional; let mut schema = TokenStream::new(); element.schema.to_schema(&mut schema)?; ts.extend(quote! { (#key, #optional, &#schema), }); @@ -538,33 +644,22 @@ impl SchemaObject { .find(|p| p.name.as_ident_str() == key) } - fn remove_property_by_ident(&mut self, key: &str) -> bool { - match self - .properties_ - .iter() - .position(|entry| entry.name.as_ident_str() == key) - { - Some(index) => { - self.properties_.remove(index); - true - } - None => false, - } - } - fn extend_properties(&mut self, new_fields: Vec) { self.properties_.extend(new_fields); self.sort_properties(); } } +#[derive(Clone)] pub struct SchemaArray { + span: Span, item: Box, } impl SchemaArray { fn try_extract_from(obj: &mut JSONObject) -> Result { Ok(Self { + span: obj.span(), item: Box::new(obj.remove_required_element("items")?.try_into()?), }) } diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs index bcadf05a..c4913c85 100644 --- a/proxmox-api-macro/src/api/structs.rs +++ b/proxmox-api-macro/src/api/structs.rs @@ -113,7 +113,10 @@ fn handle_newtype_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result Result { +fn handle_regular_struct( + attribs: JSONObject, + mut stru: syn::ItemStruct, +) -> Result { let mut schema: Schema = if attribs.is_empty() { Schema::empty_object(Span::call_site()) } else { @@ -130,7 +133,7 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result Result Result Result Result { let name = &stru.ident; @@ -274,12 +302,12 @@ fn finish_all_of_struct( )); // now check if it even has any fields - let has_fields = match &schema.item { - api::SchemaItem::Object(obj) => !obj.is_empty(), + let has_non_flattened_fields = match &schema.item { + api::SchemaItem::Object(obj) => obj.has_non_flattened_fields(), _ => panic!("object schema is not an object schema?"), }; - let (inner_schema, inner_schema_ref) = if has_fields { + let (inner_schema, inner_schema_ref) = if has_non_flattened_fields { // if it does, we need to create an "inner" schema to merge into the AllOf schema let obj_schema = { let mut ts = TokenStream::new(); @@ -332,10 +360,10 @@ fn handle_regular_field( util::infer_type(schema, &field.ty)?; - if is_option_type(&field.ty) { + if util::is_option_type(&field.ty).is_some() { if derived { - field_def.optional = true; - } else if !field_def.optional { + field_def.optional = true.into(); + } else if !field_def.optional.expect_bool() { error!(&field.ty => "non-optional Option type?"); } } @@ -343,21 +371,103 @@ fn handle_regular_field( Ok(()) } -/// Note that we cannot handle renamed imports at all here... -fn is_option_type(ty: &syn::Type) -> bool { - if let syn::Type::Path(p) = ty { - if p.qself.is_some() { - return false; - } - let segs = &p.path.segments; - match segs.len() { - 1 => return segs.last().unwrap().ident == "Option", - 2 => { - return segs.first().unwrap().ident == "std" - && segs.last().unwrap().ident == "Option" +/// To derive an `Updater` we make all fields optional and use the `Updater` derive macro with +/// a `target` parameter. +fn derive_updater( + mut stru: syn::ItemStruct, + mut schema: Schema, + original_struct: &mut syn::ItemStruct, +) -> Result { + stru.ident = Ident::new(&format!("{}Updater", stru.ident), stru.ident.span()); + + if !util::derived_items(&original_struct.attrs).any(|p| p.is_ident("Default")) { + original_struct.attrs.push(util::make_derive_attribute( + Span::call_site(), + quote::quote! { Default }, + )); + } + + original_struct.attrs.push(util::make_derive_attribute( + Span::call_site(), + quote::quote! { 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(); + + if let syn::Fields::Named(fields) = &mut stru.fields { + for field in &mut fields.named { + let field_name = field.ident.as_ref().expect("unnamed field in FieldsNamed"); + let field_name_string = field_name.to_string(); + + let field_schema = match schema.find_obj_property_by_ident_mut(&field_name_string) { + Some(obj) => obj, + None => { + error!( + field_name.span(), + "failed to find schema entry for {:?}", field_name_string, + ); + continue; + } + }; + + field_schema.optional = field.ty.clone().into(); + + let span = Span::call_site(); + let updater = syn::TypePath { + qself: Some(syn::QSelf { + lt_token: syn::token::Lt { spans: [span] }, + ty: Box::new(field.ty.clone()), + position: 4, // 'Updater' is the 4th item in the 'segments' below + as_token: Some(syn::token::As { span }), + gt_token: syn::token::Gt { spans: [span] }, + }), + path: util::make_path( + span, + true, + &["proxmox", "api", "schema", "Updatable", "Updater"], + ), + }; + field.ty = syn::Type::Path(updater); + + if field_schema.flatten_in_struct { + let updater_ty = &field.ty; + all_of_schemas.extend(quote::quote! {&#updater_ty::API_SCHEMA,}); } - _ => return false, + + if !is_empty_impl.is_empty() { + is_empty_impl.extend(quote::quote! { && }); + } + is_empty_impl.extend(quote::quote! { + self.#field_name.is_empty() + }); } } - false + + let mut output = if all_of_schemas.is_empty() { + finish_schema(schema, &stru, &stru.ident)? + } else { + finish_all_of_struct(schema, &stru, all_of_schemas)? + }; + + if !is_empty_impl.is_empty() { + output = quote::quote!( + #output + impl ::proxmox::api::schema::Updater for #updater_name { + fn is_empty(&self) -> bool { + #is_empty_impl + } + } + ); + } + + Ok(output) } diff --git a/proxmox-api-macro/src/lib.rs b/proxmox-api-macro/src/lib.rs index 2f543e76..182bfaff 100644 --- a/proxmox-api-macro/src/lib.rs +++ b/proxmox-api-macro/src/lib.rs @@ -33,6 +33,7 @@ macro_rules! bail { mod api; mod serde; +mod updater; mod util; /// Handle errors by appending a `compile_error!()` macro invocation to the original token stream. @@ -236,6 +237,20 @@ pub fn api(attr: TokenStream_1, item: TokenStream_1) -> TokenStream_1 { handle_error(item.clone(), api::api(attr.into(), item)).into() } +/// This is a dummy derive macro actually handled by `#[api]`! +#[proc_macro_derive(Updater, attributes(updater, updatable, 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 { + let _error_guard = init_local_error(); + let item: TokenStream = item.into(); + handle_error(item.clone(), updater::updatable(item).map_err(Error::from)).into() +} + thread_local!(static NON_FATAL_ERRORS: RefCell> = RefCell::new(None)); /// The local error TLS must be freed at the end of a macro as any leftover `TokenStream` (even an diff --git a/proxmox-api-macro/src/updater.rs b/proxmox-api-macro/src/updater.rs new file mode 100644 index 00000000..87501f92 --- /dev/null +++ b/proxmox-api-macro/src/updater.rs @@ -0,0 +1,286 @@ +use proc_macro2::{Ident, Span, TokenStream}; +use quote::{quote, quote_spanned}; +use syn::spanned::Spanned; + +use crate::util; + +pub(crate) fn updatable(item: TokenStream) -> Result { + let item: syn::Item = syn::parse2(item)?; + let full_span = item.span(); + 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), + 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"), + } +} + +fn no_generics(generics: syn::Generics) { + if let Some(lt) = generics.lt_token { + error!(lt => "deriving `Updatable` 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"); + } +} + +fn derive_default_updatable( + attrs: Vec, + full_span: Span, + ident: Ident, + generics: syn::Generics, +) -> Result { + 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; + + fn update_from>( + &mut self, + from: Option<#ident>, + _delete: &[T], + ) -> Result<(), ::anyhow::Error> { + if let Some(val) = from { + *self = val; + } + + Ok(()) + } + + fn try_build_from(from: Option<#ident>) -> Result { + from.ok_or_else(|| ::anyhow::format_err!("cannot build from None value")) + } + } + } +} + +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)), + }; + + let mut delete = TokenStream::new(); + let mut apply = TokenStream::new(); + let mut build = TokenStream::new(); + + for field in fields.named { + let attrs = UpdaterFieldArgs::from_attributes(field.attrs); + + let field_name = field + .ident + .as_ref() + .expect("unnamed field in named struct?"); + + let field_name_string = field_name.to_string(); + let build_err = format!("failed to build value for field '{}': {{}}", field_name_string); + if util::is_option_type(&field.ty).is_some() { + delete.extend(quote! { + #field_name_string => { self.#field_name = None; } + }); + build.extend(quote! { + #field_name: ::proxmox::api::schema::Updatable::try_build_from( + from.#field_name + ) + .map_err(|err| ::anyhow::format_err!(#build_err, err))?, + }); + } else { + build.extend(quote! { + #field_name: ::proxmox::api::schema::Updatable::try_build_from( + from.#field_name + ) + .map_err(|err| ::anyhow::format_err!(#build_err, err))?, + }); + } + + if attrs.fixed { + let error = + format!("field '{}' must not be set when updating existing data", field_name); + apply.extend(quote! { + if from.#field_name.is_some() { + ::anyhow::bail!(#error); + } + }); + } else { + apply.extend(quote! { + ::proxmox::api::schema::Updatable::update_from( + &mut self.#field_name, + from.#field_name, + delete, + )?; + }); + } + } + + if !delete.is_empty() { + delete = quote! { + for delete in delete { + match delete.as_ref() { + #delete + _ => continue, + } + } + }; + } + + Ok(quote! { + #[automatically_derived] + impl ::proxmox::api::schema::Updatable for #ident { + type Updater = #updater; + const UPDATER_IS_OPTION: bool = false; + + fn update_from>( + &mut self, + from: Self::Updater, + delete: &[T], + ) -> Result<(), ::anyhow::Error> { + #delete + #apply + Ok(()) + } + + fn try_build_from(from: Self::Updater) -> Result { + Ok(Self { + #build + }) + } + } + }) +} + +#[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"); + } + } +} + +#[derive(Default)] +struct UpdaterFieldArgs { + /// A fixed field must not be set in the `Updater` when the data is updated via `update_from`, + /// but is still required for the `build()` method. + fixed: bool, +} + +impl UpdaterFieldArgs { + fn from_attributes(attributes: Vec) -> Self { + let mut this = Self::default(); + for_attributes(attributes, "updater", |meta| this.parse_nested(meta)); + this + } + + fn parse_nested(&mut self, meta: syn::NestedMeta) -> Result<(), syn::Error> { + match meta { + syn::NestedMeta::Meta(syn::Meta::Path(path)) if path.is_ident("fixed") => { + self.fixed = true; + } + other => bail!(other => "invalid updater argument"), + } + Ok(()) + } +} + +/// 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/src/util.rs b/proxmox-api-macro/src/util.rs index 87e6414a..b4ddcc01 100644 --- a/proxmox-api-macro/src/util.rs +++ b/proxmox-api-macro/src/util.rs @@ -2,7 +2,8 @@ use std::borrow::Borrow; use std::collections::HashMap; use std::convert::TryFrom; -use proc_macro2::{Ident, Span}; +use proc_macro2::{Ident, Span, TokenStream}; +use quote::ToTokens; use syn::parse::{Parse, ParseStream}; use syn::punctuated::Punctuated; use syn::spanned::Spanned; @@ -481,19 +482,19 @@ pub fn infer_type(schema: &mut Schema, ty: &syn::Type) -> Result { if path.path.is_ident("String") { - schema.item = SchemaItem::String; + schema.item = SchemaItem::String(ty.span()); } else if path.path.is_ident("bool") { - schema.item = SchemaItem::Boolean; - } else if let Some(ty) = api::INTTYPES.iter().find(|i| path.path.is_ident(i.name)) { - schema.item = SchemaItem::Integer; - if let Some(min) = ty.minimum { + schema.item = SchemaItem::Boolean(ty.span()); + } else if let Some(info) = api::INTTYPES.iter().find(|i| path.path.is_ident(i.name)) { + schema.item = SchemaItem::Integer(ty.span()); + if let Some(min) = info.minimum { schema.add_default_property("minimum", syn::Expr::Verbatim(min.parse()?)); } - if let Some(max) = ty.maximum { + if let Some(max) = info.maximum { schema.add_default_property("maximum", syn::Expr::Verbatim(max.parse()?)); } } else if api::NUMBERNAMES.iter().any(|n| path.path.is_ident(n)) { - schema.item = SchemaItem::Number; + schema.item = SchemaItem::Number(ty.span()); } else { bail!(ty => "cannot infer parameter type from this rust type"); } @@ -531,10 +532,43 @@ pub fn is_option_type(ty: &syn::Type) -> Option<&syn::Type> { None } +pub fn make_ident_path(ident: Ident) -> syn::Path { + syn::Path { + leading_colon: None, + segments: { + let mut s = Punctuated::new(); + s.push(syn::PathSegment { + ident, + arguments: syn::PathArguments::None, + }); + s + }, + } +} + +pub fn make_path(span: Span, leading_colon: bool, path: &[&str]) -> syn::Path { + syn::Path { + leading_colon: if leading_colon { + Some(syn::token::Colon2 { + spans: [span, span], + }) + } else { + None + }, + segments: path + .into_iter() + .map(|entry| syn::PathSegment { + ident: Ident::new(entry, span), + arguments: syn::PathArguments::None, + }) + .collect(), + } +} + /// Helper to do basically what `parse_macro_input!` does, except the macro expects a /// `TokenStream_1`, and we always have a `TokenStream` from `proc_macro2`. pub struct AttrArgs { - _paren_token: syn::token::Paren, + pub paren_token: syn::token::Paren, pub args: Punctuated, } @@ -542,12 +576,19 @@ impl Parse for AttrArgs { fn parse(input: ParseStream) -> syn::Result { let content; Ok(Self { - _paren_token: syn::parenthesized!(content in input), + paren_token: syn::parenthesized!(content in input), args: Punctuated::parse_terminated(&content)?, }) } } +impl ToTokens for AttrArgs { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.paren_token + .surround(tokens, |inner| self.args.to_tokens(inner)); + } +} + /// Join an iterator over `Display` values. pub fn join(separator: &str, iter: impl Iterator) -> String where @@ -644,3 +685,129 @@ impl Into> for Maybe { } } } + +/// Helper to iterate over all the `#[derive(...)]` types found in an attribute list. +pub fn derived_items(attributes: &[syn::Attribute]) -> DerivedItems { + DerivedItems { + attributes: attributes.into_iter(), + current: None, + } +} + +/// Iterator over the types found in `#[derive(...)]` attributes. +pub struct DerivedItems<'a> { + current: Option< as IntoIterator>::IntoIter>, + attributes: std::slice::Iter<'a, syn::Attribute>, +} + +impl<'a> Iterator for DerivedItems<'a> { + type Item = syn::Path; + + fn next(&mut self) -> Option { + loop { + if let Some(current) = &mut self.current { + loop { + match current.next() { + Some(syn::NestedMeta::Meta(syn::Meta::Path(path))) => return Some(path), + Some(_) => continue, + None => { + self.current = None; + break; + } + } + } + } + + let attr = self.attributes.next()?; + if attr.style != syn::AttrStyle::Outer { + continue; + } + + match attr.parse_meta() { + Ok(syn::Meta::List(list)) if list.path.is_ident("derive") => { + self.current = Some(list.nested.into_iter()); + continue; + } + // ignore anything that isn't a `derive(...)` attribute + Ok(_) => continue, + // ignore parse errors + Err(_) => continue, + } + } + } +} + +/// Helper to iterate over all the `#[derive(...)]` types found in an attribute list. +pub fn retain_derived_items(attributes: &mut Vec, mut func: F) +where + F: FnMut(&syn::Path) -> bool, +{ + use syn::punctuated::Pair; + + let capacity = attributes.len(); + for mut attr in std::mem::replace(attributes, Vec::with_capacity(capacity)) { + if attr.style != syn::AttrStyle::Outer { + attributes.push(attr); + continue; + } + + if !attr.path.is_ident("derive") { + attributes.push(attr); + continue; + } + + let mut args: AttrArgs = match syn::parse2(attr.tokens.clone()) { + Ok(args) => args, + Err(_) => { + // if we can't parse it, we don't care + attributes.push(attr); + continue; + } + }; + + for arg in std::mem::take(&mut args.args).into_pairs() { + match arg { + Pair::Punctuated(item, punct) => { + if let syn::NestedMeta::Meta(syn::Meta::Path(path)) = &item { + if !func(path) { + continue; + } + } + args.args.push_value(item); + args.args.push_punct(punct); + } + Pair::End(item) => { + if let syn::NestedMeta::Meta(syn::Meta::Path(path)) = &item { + if !func(path) { + continue; + } + } + args.args.push_value(item); + } + } + } + + if !args.args.is_empty() { + attr.tokens = args.into_token_stream(); + attributes.push(attr); + } + } +} + +pub fn make_attribute(span: Span, path: syn::Path, tokens: TokenStream) -> syn::Attribute { + syn::Attribute { + pound_token: syn::token::Pound { spans: [span] }, + style: syn::AttrStyle::Outer, + bracket_token: syn::token::Bracket { span }, + path, + tokens, + } +} + +pub fn make_derive_attribute(span: Span, content: TokenStream) -> syn::Attribute { + make_attribute( + span, + make_ident_path(Ident::new("derive", span)), + quote::quote! { (#content) }, + ) +} diff --git a/proxmox-api-macro/tests/updater.rs b/proxmox-api-macro/tests/updater.rs new file mode 100644 index 00000000..68cdce60 --- /dev/null +++ b/proxmox-api-macro/tests/updater.rs @@ -0,0 +1,330 @@ +#[cfg(not(feature = "noserde"))] +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +use proxmox::api::schema::Updater; +use proxmox_api_macro::{api, Updatable, Updater}; + +#[api] +/// An example of a simple struct type. +#[cfg_attr(not(feature = "noserde"), derive(Deserialize, Serialize))] +#[derive(Debug, PartialEq, Updater)] +pub struct Simple { + /// A test string. + one: String, + + /// An optional auto-derived value for testing: + #[serde(skip_serializing_if = "Option::is_empty")] + opt: Option, +} + +#[api( + properties: { + simple: { type: Simple }, + }, +)] +/// A second struct so we can test flattening. +#[cfg_attr(not(feature = "noserde"), derive(Deserialize, Serialize))] +#[derive(Debug, PartialEq, Updater)] +pub struct Complex { + /// An extra field not part of the flattened struct. + extra: String, + + #[serde(flatten)] + simple: Simple, +} + +#[api( + properties: { + simple: { + type: Simple, + optional: true, + }, + }, +)] +/// One of the baaaad cases. +#[cfg_attr(not(feature = "noserde"), derive(Deserialize, Serialize))] +#[derive(Debug, PartialEq, Updater)] +pub struct SuperComplex { + /// An extra field not part of the flattened struct. + extra: String, + + #[serde(skip_serializing_if = "Updater::is_empty")] + simple: Option, +} + +#[api( + properties: { + complex: { type: Complex }, + }, +)] +/// Something with "fixed" values we cannot update but require for creation. +#[cfg_attr(not(feature = "noserde"), derive(Deserialize, Serialize))] +#[derive(Debug, PartialEq, Updater)] +pub struct Creatable { + /// An ID which cannot be changed later. + #[updater(fixed)] + id: String, + + /// Some parameter we're allowed to change with an updater. + name: String, + + /// Optional additional information. + #[serde(skip_serializing_if = "Updater::is_empty", default)] + info: Option, + + /// Optional additional information 2. + #[serde(skip_serializing_if = "Updater::is_empty", default)] + info2: Option, + + /// Super complex additional data + #[serde(flatten)] + complex: Complex, +} + +struct RpcEnv; +impl proxmox::api::RpcEnvironment for RpcEnv { + fn result_attrib_mut(&mut self) -> &mut Value { + panic!("result_attrib_mut called"); + } + + fn result_attrib(&self) -> &Value { + panic!("result_attrib called"); + } + + /// The environment type + fn env_type(&self) -> proxmox::api::RpcEnvironmentType { + panic!("env_type called"); + } + + /// Set authentication id + fn set_auth_id(&mut self, user: Option) { + let _ = user; + panic!("set_auth_id called"); + } + + /// Get authentication id + fn get_auth_id(&self) -> Option { + panic!("get_auth_id called"); + } +} + +mod test_creatable { + use anyhow::{bail, Error}; + use serde_json::json; + + use proxmox::api::schema::Updatable; + use proxmox_api_macro::api; + + use super::*; + + static mut TEST_OBJECT: Option = None; + + #[api( + input: { + properties: { + thing: { flatten: true, type: CreatableUpdater }, + }, + }, + )] + /// Test method to create an object. + /// + /// Returns: the object's ID. + pub fn create_thing(thing: CreatableUpdater) -> Result { + if unsafe { TEST_OBJECT.is_some() } { + bail!("object exists"); + } + + let obj = Creatable::try_build_from(thing)?; + let id = obj.id.clone(); + + unsafe { + TEST_OBJECT = Some(obj); + } + + Ok(id) + } + + #[api( + input: { + properties: { + thing: { flatten: true, type: CreatableUpdater }, + delete: { + optional: true, + description: "list of properties to delete", + type: Array, + items: { + description: "field name to delete", + type: String, + }, + }, + }, + }, + )] + /// Test method to update an object. + pub fn update_thing(thing: CreatableUpdater, delete: Option>) -> Result<(), Error> { + let delete = delete.unwrap_or_default(); + match unsafe { &mut TEST_OBJECT } { + Some(obj) => obj.update_from(thing, &delete), + None => bail!("object has not been created yet"), + } + } + + #[test] + fn test() { + let _ = api_function_create_thing( + json!({ "name": "The Name" }), + &API_METHOD_CREATE_THING, + &mut RpcEnv, + ) + .expect_err("create_thing should fail without an ID"); + + let _ = api_function_create_thing( + json!({ "id": "Id1" }), + &API_METHOD_CREATE_THING, + &mut RpcEnv, + ) + .expect_err("create_thing should fail without a name"); + + let value = api_function_create_thing( + json!({ + "id": "Id1", + "name": "The Name", + "extra": "Extra Info", + "one": "Part of Simple", + "info2": "More Info 2", + }), + &API_METHOD_CREATE_THING, + &mut RpcEnv, + ) + .expect("create_thing should work"); + assert_eq!(value, "Id1"); + assert_eq!( + unsafe { &TEST_OBJECT }, + &Some(Creatable { + id: "Id1".to_string(), + name: "The Name".to_string(), + info: None, + info2: Some("More Info 2".to_string()), + complex: Complex { + extra: "Extra Info".to_string(), + simple: Simple { + one: "Part of Simple".to_string(), + opt: None, + }, + }, + }), + ); + + let _ = api_function_update_thing( + json!({ + "id": "Poop", + }), + &API_METHOD_UPDATE_THING, + &mut RpcEnv, + ) + .expect_err("shouldn't be allowed to update the ID"); + + let _ = api_function_update_thing( + json!({ + "info": "Updated Info", + "delete": ["info2"], + }), + &API_METHOD_UPDATE_THING, + &mut RpcEnv, + ) + .expect("should be allowed to update the optional field"); + + assert_eq!( + unsafe { &TEST_OBJECT }, + &Some(Creatable { + id: "Id1".to_string(), + name: "The Name".to_string(), + info: Some("Updated Info".to_string()), + info2: None, + complex: Complex { + extra: "Extra Info".to_string(), + simple: Simple { + one: "Part of Simple".to_string(), + opt: None, + }, + }, + }), + ); + + let _ = api_function_update_thing( + json!({ + "extra": "Partial flatten update", + }), + &API_METHOD_UPDATE_THING, + &mut RpcEnv, + ) + .expect("should be allowed to update the parts of a flattened field"); + assert_eq!( + unsafe { &TEST_OBJECT }, + &Some(Creatable { + id: "Id1".to_string(), + name: "The Name".to_string(), + info: Some("Updated Info".to_string()), + info2: None, + complex: Complex { + extra: "Partial flatten update".to_string(), + simple: Simple { + one: "Part of Simple".to_string(), + opt: None, + }, + }, + }), + ); + + let _ = api_function_update_thing( + json!({ + "opt": "Deeply nested optional update.", + }), + &API_METHOD_UPDATE_THING, + &mut RpcEnv, + ) + .expect("should be allowed to update the parts of a deeply nested struct"); + assert_eq!( + unsafe { &TEST_OBJECT }, + &Some(Creatable { + id: "Id1".to_string(), + name: "The Name".to_string(), + info: Some("Updated Info".to_string()), + info2: None, + complex: Complex { + extra: "Partial flatten update".to_string(), + simple: Simple { + one: "Part of Simple".to_string(), + opt: Some("Deeply nested optional update.".to_string()), + }, + }, + }), + ); + + let _ = api_function_update_thing( + json!({ + "delete": ["opt"], + }), + &API_METHOD_UPDATE_THING, + &mut RpcEnv, + ) + .expect("should be allowed to remove parts of a deeply nested struct"); + assert_eq!( + unsafe { &TEST_OBJECT }, + &Some(Creatable { + id: "Id1".to_string(), + name: "The Name".to_string(), + info: Some("Updated Info".to_string()), + info2: None, + complex: Complex { + extra: "Partial flatten update".to_string(), + simple: Simple { + one: "Part of Simple".to_string(), + opt: None, + }, + }, + }), + ); + } +}