more updater cleanups

* Updatable is now named UpdaterType
* UPDATER_IS_OPTION is now assumed to always be true
    While an updater can be a non-optional struct, being an
    updater, all its fields are also Updaters, so after
    expanding all levels of nesting, the resulting list of
    fields can only contain optional values.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
This commit is contained in:
Wolfgang Bumiller 2021-08-13 11:11:27 +02:00
parent cb9d57b453
commit 744d69c2ab
10 changed files with 123 additions and 222 deletions

View File

@ -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<Self>;
const UPDATER_IS_OPTION: bool = true;
}
})
}

View File

@ -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 }),
}
}
}

View File

@ -62,9 +62,8 @@ fn handle_unit_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<Toke
let name = &stru.ident;
let mut schema = finish_schema(schema, &stru, name)?;
schema.extend(quote_spanned! { name.span() =>
impl ::proxmox::api::schema::Updatable for #name {
impl ::proxmox::api::schema::UpdaterType for #name {
type Updater = Option<Self>;
const UPDATER_IS_OPTION: bool = true;
}
});
@ -404,6 +403,7 @@ fn derive_updater(
mut schema: Schema,
original_struct: &mut syn::ItemStruct,
) -> Result<TokenStream, Error> {
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() {

View File

@ -231,7 +231,7 @@ fn router_do(item: TokenStream) -> Result<TokenStream, Error> {
# 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<TokenStream, Error> {
#[api]
/// An example of a simple struct type.
pub struct MyTypeUpdater {
one: Option<String>, // really <String as Updatable>::Updater
one: Option<String>, // really <String as UpdaterType>::Updater
#[serde(skip_serializing_if = "Option::is_none")]
opt: Option<String>, // really <Option<String> as Updatable>::Updater
opt: Option<String>, // really <Option<String> as UpdaterType>::Updater
}
impl Updater for MyTypeUpdater {
@ -277,36 +277,8 @@ fn router_do(item: TokenStream) -> Result<TokenStream, Error> {
}
}
impl Updatable for MyType {
impl UpdaterType for MyType {
type Updater = MyTypeUpdater;
fn update_from<T>(&mut self, from: MyTypeUpdater, delete: &[T]) -> Result<(), Error>
where
T: AsRef<str>,
{
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<Self, Error> {
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<Self>`.
#[proc_macro_derive(Updatable, attributes(updatable, serde))]
pub fn derive_updatable(item: TokenStream_1) -> TokenStream_1 {
/// Create the default `UpdaterType` implementation as an `Option<Self>`.
#[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<Option<TokenStream>> = RefCell::new(None));

View File

@ -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<TokenStream, syn::Error> {
pub(crate) fn updater_type(item: TokenStream) -> Result<TokenStream, syn::Error> {
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<syn::Attribute>,
fn derive_updater_type(
full_span: Span,
ident: Ident,
generics: syn::Generics,
) -> Result<TokenStream, syn::Error> {
) -> 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<Self>;
}
}
}
fn derive_named_struct_updatable(
attrs: Vec<syn::Attribute>,
full_span: Span,
ident: Ident,
generics: syn::Generics,
_fields: syn::FieldsNamed,
) -> Result<TokenStream, syn::Error> {
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<syn::Type>,
}
impl UpdatableArgs {
fn from_attributes(attributes: Vec<syn::Attribute>) -> 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<F>(attributes: Vec<syn::Attribute>, 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<Item = syn::Attribute>,
) -> impl Iterator<Item = syn::Meta> {
attributes.into_iter().filter_map(|attr| {
if attr.style != syn::AttrStyle::Outer {
return None;
}
attr.parse_meta().ok()
})
}

View File

@ -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();

View File

@ -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.

View File

@ -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;

View File

@ -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>(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,
<Custom as UpdaterType>::Updater,
Option<Custom>
);
#[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<String>,
}
#[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: 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, &<Option<Custom> as ApiType>::API_SCHEMA),
(
"extra",
true,
&::proxmox::api::schema::StringSchema::new("An extra field.").schema(),
),
(
"simple",
true,
//&<<Simple as UpdaterType>::Updater as ApiType>::API_SCHEMA,
&SimpleUpdater::API_SCHEMA,
),
],
)
.schema();
assert_eq!(TEST_SCHEMA, SuperComplexUpdater::API_SCHEMA);
}

View File

@ -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<Self>`. 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<T>` 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<Self>;
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<T> Updatable for Option<T>
impl<T> UpdaterType for Option<T>
where
T: Updatable,
T: UpdaterType,
{
type Updater = T::Updater;
const UPDATER_IS_OPTION: bool = true;
}
pub trait ApiType {