public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema
@ 2020-12-18 11:25 Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 01/18] formatting fixup Wolfgang Bumiller
                   ` (20 more replies)
  0 siblings, 21 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

This series extends the schema and #[api] macro a bit:

* Optional return types (requested by Dietmar)

    Changes `ApiMethod` to allow marking the return type as optional.

    Affects mostly generated documentation & api macro in the proxmox crate.
    The generated documentation can probably be improved.
    How we utilize this is up to the user (backup crate).

* AllOf schema (we really do need this one way or another, so here's
  aproposed mechanism)

    This is a "limited" variant of the `AllOf` schema found in JSON
    Schema & openapi specs.

    This affects a lot of code paths which previously directly
    interacted with an `ObjectSchema`.

    Changes `ApiMethod` to take a `ParameterSchema` instead of an
    `ObjectSchema`, which is a subset of `Schema` consisting of only
    `ObjectSchema` and `AllOfSchema`. All three of those now also
    implement the `ObjectSchemaType` trait containing all the common
    functionality (property lookup, property iteration, "additional
    property" query, ...).

    As for the `#[api]` macro side, we don't allow directly writing an
    `allOf` schema instead of an object schema inside the macro.
    Only 2 cases are supported:
    * For `struct`s we act on `#[serde(flatten)]`, so any struct which
      is an API type can now `flatten` other types into it via serde
      (takes care of deserialization), while still staying a valid
      `#[api]` type.
    * For api *methods*, parameters may now be marked as `flatten`
      directly in the object schema:
          ```
          (...)
          properties: {
              "argname": {
                  type: Foo,
                  flatten: true,
              }
              (...)
          }
          (...)
          ```

This also includes 2 patches for temporary changelog updates mentioning
the breaking changes.

The backup side of this series also extends the `verify-api` test to
ensure we don't have duplicate keys in parameters after "flattening"
them.

Wolfgang Bumiller (18):
  formatting fixup
  schema: support optional return values
  api-macro: support optional return values
  schema: support AllOf schemas
  schema: allow AllOf schema as method parameter
  api-macro: add 'flatten' to SerdeAttrib
  api-macro: forbid flattened fields
  api-macro: add more standard Maybe methods
  api-macro: suport AllOf on structs
  schema: ExtractValueDeserializer
  api-macro: object schema entry tuple -> struct
  api-macro: more tuple refactoring
  api-macro: factor parameter extraction into a function
  api-macro: support flattened parameters
  schema: ParameterSchema at 'api' level
  proxmox: temporary d/changelog update
  macro: temporary d/changelog update
  proxmox changelog update

 proxmox-api-macro/debian/changelog    |  10 +
 proxmox-api-macro/src/api/method.rs   | 513 +++++++++++++++++++-------
 proxmox-api-macro/src/api/mod.rs      | 136 +++++--
 proxmox-api-macro/src/api/structs.rs  | 142 ++++++-
 proxmox-api-macro/src/serde.rs        |  11 +-
 proxmox-api-macro/src/util.rs         |  25 +-
 proxmox-api-macro/tests/allof.rs      | 245 ++++++++++++
 proxmox-api-macro/tests/api1.rs       |  10 +-
 proxmox-api-macro/tests/ext-schema.rs |   2 +-
 proxmox-api-macro/tests/types.rs      |   7 +-
 proxmox/debian/changelog              |  18 +
 proxmox/src/api/cli/command.rs        |   2 +-
 proxmox/src/api/cli/completion.rs     |  11 +-
 proxmox/src/api/cli/format.rs         |  22 +-
 proxmox/src/api/cli/getopts.rs        |  19 +-
 proxmox/src/api/cli/text_table.rs     |  49 ++-
 proxmox/src/api/de.rs                 | 270 ++++++++++++++
 proxmox/src/api/format.rs             |  29 +-
 proxmox/src/api/mod.rs                |   5 +-
 proxmox/src/api/router.rs             | 117 +++++-
 proxmox/src/api/schema.rs             | 178 ++++++++-
 proxmox/src/api/section_config.rs     |   2 +-
 proxmox/src/tools/io/read.rs          |  16 +-
 23 files changed, 1569 insertions(+), 270 deletions(-)
 create mode 100644 proxmox-api-macro/tests/allof.rs
 create mode 100644 proxmox/src/api/de.rs

Wolfgang Bumiller (2):
  adaptions for proxmox 0.9 and proxmox-api-macro 0.3
  tests: verify-api: check AllOf schemas

 src/api2/admin/datastore.rs                   |  8 ++--
 src/bin/proxmox-backup-client.rs              | 12 ++---
 src/bin/proxmox-backup-manager.rs             | 12 ++---
 src/bin/proxmox-tape.rs                       |  4 +-
 src/bin/proxmox_backup_client/benchmark.rs    |  5 +-
 src/bin/proxmox_backup_client/key.rs          | 10 +++-
 src/bin/proxmox_backup_client/snapshot.rs     |  9 ++--
 src/bin/proxmox_backup_client/task.rs         |  4 +-
 src/bin/proxmox_backup_manager/acl.rs         |  2 +-
 src/bin/proxmox_backup_manager/datastore.rs   |  4 +-
 src/bin/proxmox_backup_manager/disk.rs        |  8 ++--
 src/bin/proxmox_backup_manager/dns.rs         |  2 +-
 src/bin/proxmox_backup_manager/network.rs     |  2 +-
 src/bin/proxmox_backup_manager/remote.rs      |  4 +-
 .../proxmox_backup_manager/subscription.rs    |  2 +-
 src/bin/proxmox_backup_manager/sync.rs        |  4 +-
 src/bin/proxmox_backup_manager/user.rs        |  4 +-
 src/bin/proxmox_tape/changer.rs               |  8 ++--
 src/bin/proxmox_tape/drive.rs                 |  6 +--
 src/bin/proxmox_tape/media.rs                 |  2 +-
 src/bin/proxmox_tape/pool.rs                  |  4 +-
 src/server/rest.rs                            |  9 ++--
 tests/verify-api.rs                           | 46 +++++++++++++++++--
 23 files changed, 110 insertions(+), 61 deletions(-)

-- 
2.20.1




^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 01/18] formatting fixup
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 02/18] schema: support optional return values Wolfgang Bumiller
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/src/tools/io/read.rs | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/proxmox/src/tools/io/read.rs b/proxmox/src/tools/io/read.rs
index 9bba54b..eed96b7 100644
--- a/proxmox/src/tools/io/read.rs
+++ b/proxmox/src/tools/io/read.rs
@@ -298,17 +298,21 @@ impl<R: io::Read> ReadExt for R {
         loop {
             match self.read(&mut buf) {
                 Ok(0) => {
-                    if read_bytes == 0 { return Ok(false); }
+                    if read_bytes == 0 {
+                        return Ok(false);
+                    }
                     return Err(io::Error::new(
                         io::ErrorKind::UnexpectedEof,
-                        "failed to fill whole buffer")
-                    );
+                        "failed to fill whole buffer",
+                    ));
                 }
-                Ok(n) =>  {
+                Ok(n) => {
                     let tmp = buf;
                     buf = &mut tmp[n..];
                     read_bytes += n;
-                    if buf.is_empty() { return Ok(true); }
+                    if buf.is_empty() {
+                        return Ok(true);
+                    }
                 }
                 Err(ref e) if e.kind() == std::io::ErrorKind::Interrupted => {}
                 Err(e) => return Err(e),
@@ -318,7 +322,7 @@ impl<R: io::Read> ReadExt for R {
 
     fn skip_to_end(&mut self) -> io::Result<usize> {
         let mut skipped_bytes = 0;
-        let mut buf = unsafe { vec::uninitialized(32*1024) };
+        let mut buf = unsafe { vec::uninitialized(32 * 1024) };
         loop {
             match self.read(&mut buf) {
                 Ok(0) => return Ok(skipped_bytes),
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 02/18] schema: support optional return values
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 01/18] formatting fixup Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 03/18] api-macro: " Wolfgang Bumiller
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/src/api/cli/format.rs |  9 +++++++--
 proxmox/src/api/format.rs     | 13 +++++++++---
 proxmox/src/api/router.rs     | 37 ++++++++++++++++++++++++++++++-----
 3 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/proxmox/src/api/cli/format.rs b/proxmox/src/api/cli/format.rs
index 61d530a..a4e7c02 100644
--- a/proxmox/src/api/cli/format.rs
+++ b/proxmox/src/api/cli/format.rs
@@ -5,6 +5,7 @@ use serde_json::Value;
 use std::collections::HashSet;
 
 use crate::api::format::*;
+use crate::api::router::ReturnType;
 use crate::api::schema::*;
 
 use super::{value_to_text, TableFormatOptions};
@@ -32,16 +33,20 @@ pub fn format_and_print_result(result: &Value, output_format: &str) {
 /// formatted tables with borders.
 pub fn format_and_print_result_full(
     result: &mut Value,
-    schema: &Schema,
+    return_type: &ReturnType,
     output_format: &str,
     options: &TableFormatOptions,
 ) {
+    if return_type.optional && result.is_null() {
+        return;
+    }
+
     if output_format == "json-pretty" {
         println!("{}", serde_json::to_string_pretty(&result).unwrap());
     } else if output_format == "json" {
         println!("{}", serde_json::to_string(&result).unwrap());
     } else if output_format == "text" {
-        if let Err(err) = value_to_text(std::io::stdout(), result, schema, options) {
+        if let Err(err) = value_to_text(std::io::stdout(), result, &return_type.schema, options) {
             eprintln!("unable to format result: {}", err);
         }
     } else {
diff --git a/proxmox/src/api/format.rs b/proxmox/src/api/format.rs
index 6916b26..eac2214 100644
--- a/proxmox/src/api/format.rs
+++ b/proxmox/src/api/format.rs
@@ -4,6 +4,7 @@ use anyhow::Error;
 
 use std::io::Write;
 
+use crate::api::router::ReturnType;
 use crate::api::schema::*;
 use crate::api::{ApiHandler, ApiMethod};
 
@@ -197,8 +198,14 @@ fn dump_api_parameters(param: &ObjectSchema) -> String {
     res
 }
 
-fn dump_api_return_schema(schema: &Schema) -> String {
-    let mut res = String::from("*Returns*: ");
+fn dump_api_return_schema(returns: &ReturnType) -> String {
+    let schema = &returns.schema;
+
+    let mut res = if returns.optional {
+        "*Returns* (optionally): ".to_string()
+    } else {
+        "*Returns*: ".to_string()
+    };
 
     let type_text = get_schema_type_text(schema, ParameterDisplayStyle::Config);
     res.push_str(&format!("**{}**\n\n", type_text));
@@ -243,7 +250,7 @@ fn dump_method_definition(method: &str, path: &str, def: Option<&ApiMethod>) ->
         Some(api_method) => {
             let param_descr = dump_api_parameters(api_method.parameters);
 
-            let return_descr = dump_api_return_schema(api_method.returns);
+            let return_descr = dump_api_return_schema(&api_method.returns);
 
             let mut method = method;
 
diff --git a/proxmox/src/api/router.rs b/proxmox/src/api/router.rs
index f08f9a8..9fb9ec1 100644
--- a/proxmox/src/api/router.rs
+++ b/proxmox/src/api/router.rs
@@ -398,6 +398,33 @@ pub struct ApiAccess {
     pub permission: &'static Permission,
 }
 
+#[cfg_attr(feature = "test-harness", derive(Eq, PartialEq))]
+pub struct ReturnType {
+    /// A return type may be optional, meaning the method may return null or some fixed data.
+    ///
+    /// If true, the return type in pseudo openapi terms would be `"oneOf": [ "null", "T" ]`.
+    pub optional: bool,
+
+    /// The method's return type.
+    pub schema: &'static schema::Schema,
+}
+
+impl std::fmt::Debug for ReturnType {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if self.optional {
+            write!(f, "optional {:?}", self.schema)
+        } else {
+            write!(f, "{:?}", self.schema)
+        }
+    }
+}
+
+impl ReturnType {
+    pub const fn new(optional: bool, schema: &'static Schema) -> Self {
+        Self { optional, schema }
+    }
+}
+
 /// This struct defines a synchronous API call which returns the result as json `Value`
 #[cfg_attr(feature = "test-harness", derive(Eq, PartialEq))]
 pub struct ApiMethod {
@@ -410,7 +437,7 @@ pub struct ApiMethod {
     /// Parameter type Schema
     pub parameters: &'static schema::ObjectSchema,
     /// Return type Schema
-    pub returns: &'static schema::Schema,
+    pub returns: ReturnType,
     /// Handler function
     pub handler: &'static ApiHandler,
     /// Access Permissions
@@ -433,7 +460,7 @@ impl ApiMethod {
         Self {
             parameters,
             handler,
-            returns: &NULL_SCHEMA,
+            returns: ReturnType::new(false, &NULL_SCHEMA),
             protected: false,
             reload_timezone: false,
             access: ApiAccess {
@@ -447,7 +474,7 @@ impl ApiMethod {
         Self {
             parameters,
             handler: &DUMMY_HANDLER,
-            returns: &NULL_SCHEMA,
+            returns: ReturnType::new(false, &NULL_SCHEMA),
             protected: false,
             reload_timezone: false,
             access: ApiAccess {
@@ -457,8 +484,8 @@ impl ApiMethod {
         }
     }
 
-    pub const fn returns(mut self, schema: &'static Schema) -> Self {
-        self.returns = schema;
+    pub const fn returns(mut self, returns: ReturnType) -> Self {
+        self.returns = returns;
 
         self
     }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 03/18] api-macro: support optional return values
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 01/18] formatting fixup Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 02/18] schema: support optional return values Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 04/18] schema: support AllOf schemas Wolfgang Bumiller
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

The return specification can now include an `optional`
field.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/method.rs   | 88 +++++++++++++++++++++------
 proxmox-api-macro/src/api/mod.rs      |  6 +-
 proxmox-api-macro/src/api/structs.rs  |  7 +--
 proxmox-api-macro/src/util.rs         |  4 +-
 proxmox-api-macro/tests/api1.rs       | 10 +--
 proxmox-api-macro/tests/ext-schema.rs |  2 +-
 proxmox-api-macro/tests/types.rs      |  7 ++-
 7 files changed, 89 insertions(+), 35 deletions(-)

diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index 0d05cbb..3e85d8c 100644
--- a/proxmox-api-macro/src/api/method.rs
+++ b/proxmox-api-macro/src/api/method.rs
@@ -20,6 +20,66 @@ use syn::Ident;
 use super::{Schema, SchemaItem};
 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
+/// schema.
+pub struct ReturnType {
+    /// If optional, we store `Some(span)`, otherwise `None`.
+    optional: Option<Span>,
+
+    schema: Schema,
+}
+
+impl ReturnType {
+    fn to_schema(&self, ts: &mut TokenStream) -> Result<(), Error> {
+        let optional = match self.optional {
+            Some(span) => quote_spanned! { span => true },
+            None => quote! { false },
+        };
+
+        let mut out = TokenStream::new();
+        self.schema.to_schema(&mut out)?;
+
+        ts.extend(quote! {
+            ::proxmox::api::router::ReturnType::new( #optional , &#out )
+        });
+        Ok(())
+    }
+}
+
+impl TryFrom<JSONValue> for ReturnType {
+    type Error = syn::Error;
+
+    fn try_from(value: JSONValue) -> Result<Self, syn::Error> {
+        Self::try_from(value.into_object("a return type definition")?)
+    }
+}
+
+/// To go from a `JSONObject` to a `ReturnType` we first extract the `optional` flag, then forward
+/// to the `Schema` parser.
+impl TryFrom<JSONObject> for ReturnType {
+    type Error = syn::Error;
+
+    fn try_from(mut obj: JSONObject) -> Result<Self, syn::Error> {
+        let optional = match obj.remove("optional") {
+            Some(value) => {
+                let span = value.span();
+                let is_optional: bool = value.try_into()?;
+                if is_optional {
+                    Some(span)
+                } else {
+                    None
+                }
+            }
+            None => None,
+        };
+
+        Ok(Self {
+            optional,
+            schema: obj.try_into()?,
+        })
+    }
+}
+
 /// Parse `input`, `returns` and `protected` attributes out of an function annotated
 /// with an `#[api]` attribute and produce a `const ApiMethod` named after the function.
 ///
@@ -35,7 +95,7 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
         },
     };
 
-    let mut returns_schema: Option<Schema> = attribs
+    let mut return_type: Option<ReturnType> = attribs
         .remove("returns")
         .map(|ret| ret.into_object("return schema definition")?.try_into())
         .transpose()?;
@@ -78,7 +138,7 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
     let (doc_comment, doc_span) = util::get_doc_comments(&func.attrs)?;
     util::derive_descriptions(
         &mut input_schema,
-        &mut returns_schema,
+        return_type.as_mut().map(|rs| &mut rs.schema),
         &doc_comment,
         doc_span,
     )?;
@@ -89,7 +149,7 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
     let is_async = func.sig.asyncness.is_some();
     let api_func_name = handle_function_signature(
         &mut input_schema,
-        &mut returns_schema,
+        &mut return_type,
         &mut func,
         &mut wrapper_ts,
         &mut default_consts,
@@ -110,10 +170,6 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
         &format!("API_METHOD_{}", func_name.to_string().to_uppercase()),
         func.sig.ident.span(),
     );
-    let return_schema_name = Ident::new(
-        &format!("API_RETURN_SCHEMA_{}", func_name.to_string().to_uppercase()),
-        func.sig.ident.span(),
-    );
     let input_schema_name = Ident::new(
         &format!(
             "API_PARAMETER_SCHEMA_{}",
@@ -122,15 +178,11 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
         func.sig.ident.span(),
     );
 
-    let mut returns_schema_definition = TokenStream::new();
     let mut returns_schema_setter = TokenStream::new();
-    if let Some(schema) = returns_schema {
+    if let Some(return_type) = return_type {
         let mut inner = TokenStream::new();
-        schema.to_schema(&mut inner)?;
-        returns_schema_definition = quote_spanned! { func.sig.span() =>
-            pub const #return_schema_name: ::proxmox::api::schema::Schema = #inner;
-        };
-        returns_schema_setter = quote! { .returns(&#return_schema_name) };
+        return_type.to_schema(&mut inner)?;
+        returns_schema_setter = quote! { .returns(#inner) };
     }
 
     let api_handler = if is_async {
@@ -140,8 +192,6 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
     };
 
     Ok(quote_spanned! { func.sig.span() =>
-        #returns_schema_definition
-
         pub const #input_schema_name: ::proxmox::api::schema::ObjectSchema =
             #input_schema;
 
@@ -189,7 +239,7 @@ fn check_input_type(input: &syn::FnArg) -> Result<(&syn::PatType, &syn::PatIdent
 
 fn handle_function_signature(
     input_schema: &mut Schema,
-    returns_schema: &mut Option<Schema>,
+    return_type: &mut Option<ReturnType>,
     func: &mut syn::ItemFn,
     wrapper_ts: &mut TokenStream,
     default_consts: &mut TokenStream,
@@ -322,7 +372,7 @@ fn handle_function_signature(
 
     create_wrapper_function(
         input_schema,
-        returns_schema,
+        return_type,
         param_list,
         func,
         wrapper_ts,
@@ -379,7 +429,7 @@ fn is_value_type(ty: &syn::Type) -> bool {
 
 fn create_wrapper_function(
     _input_schema: &Schema,
-    _returns_schema: &Option<Schema>,
+    _returns_schema: &Option<ReturnType>,
     param_list: Vec<(FieldName, ParameterType)>,
     func: &syn::ItemFn,
     wrapper_ts: &mut TokenStream,
diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs
index 81d9177..815e167 100644
--- a/proxmox-api-macro/src/api/mod.rs
+++ b/proxmox-api-macro/src/api/mod.rs
@@ -15,7 +15,7 @@ use proc_macro2::{Span, TokenStream};
 use quote::{quote, quote_spanned};
 use syn::parse::{Parse, ParseStream, Parser};
 use syn::spanned::Spanned;
-use syn::{ExprPath, Ident};
+use syn::{Expr, ExprPath, Ident};
 
 use crate::util::{FieldName, JSONObject, JSONValue, Maybe};
 
@@ -217,7 +217,7 @@ pub enum SchemaItem {
     Object(SchemaObject),
     Array(SchemaArray),
     ExternType(ExprPath),
-    ExternSchema(ExprPath),
+    ExternSchema(Expr),
     Inferred(Span),
 }
 
@@ -225,7 +225,7 @@ impl SchemaItem {
     /// 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<Self, syn::Error> {
-        if let Some(ext) = obj.remove("schema").map(ExprPath::try_from).transpose()? {
+        if let Some(ext) = obj.remove("schema").map(Expr::try_from).transpose()? {
             return Ok(SchemaItem::ExternSchema(ext));
         }
 
diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs
index 887ffaa..71cdc8a 100644
--- a/proxmox-api-macro/src/api/structs.rs
+++ b/proxmox-api-macro/src/api/structs.rs
@@ -41,7 +41,7 @@ pub fn handle_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<Token
 fn get_struct_description(schema: &mut Schema, stru: &syn::ItemStruct) -> Result<(), Error> {
     if schema.description.is_none() {
         let (doc_comment, doc_span) = util::get_doc_comments(&stru.attrs)?;
-        util::derive_descriptions(schema, &mut None, &doc_comment, doc_span)?;
+        util::derive_descriptions(schema, None, &doc_comment, doc_span)?;
     }
 
     Ok(())
@@ -184,8 +184,7 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
         let bad_fields = util::join(", ", schema_fields.keys());
         error!(
             schema.span,
-            "struct does not contain the following fields: {}",
-            bad_fields
+            "struct does not contain the following fields: {}", bad_fields
         );
     }
 
@@ -211,7 +210,7 @@ fn handle_regular_field(
 
     if schema.description.is_none() {
         let (doc_comment, doc_span) = util::get_doc_comments(&field.attrs)?;
-        util::derive_descriptions(schema, &mut None, &doc_comment, doc_span)?;
+        util::derive_descriptions(schema, None, &doc_comment, doc_span)?;
     }
 
     util::infer_type(schema, &field.ty)?;
diff --git a/proxmox-api-macro/src/util.rs b/proxmox-api-macro/src/util.rs
index 3cd29a0..e74e04b 100644
--- a/proxmox-api-macro/src/util.rs
+++ b/proxmox-api-macro/src/util.rs
@@ -428,7 +428,7 @@ pub fn get_doc_comments(attributes: &[syn::Attribute]) -> Result<(String, Span),
 
 pub fn derive_descriptions(
     input_schema: &mut Schema,
-    returns_schema: &mut Option<Schema>,
+    returns_schema: Option<&mut Schema>,
     doc_comment: &str,
     doc_span: Span,
 ) -> Result<(), Error> {
@@ -447,7 +447,7 @@ pub fn derive_descriptions(
     }
 
     if let Some(second) = parts.next() {
-        if let Some(ref mut returns_schema) = returns_schema {
+        if let Some(returns_schema) = returns_schema {
             if returns_schema.description.is_none() {
                 returns_schema.description =
                     Maybe::Derived(syn::LitStr::new(second.trim(), doc_span));
diff --git a/proxmox-api-macro/tests/api1.rs b/proxmox-api-macro/tests/api1.rs
index ff37c74..88adb40 100644
--- a/proxmox-api-macro/tests/api1.rs
+++ b/proxmox-api-macro/tests/api1.rs
@@ -82,7 +82,8 @@ fn create_ticket_schema_check() {
             ],
         ),
     )
-    .returns(
+    .returns(::proxmox::api::router::ReturnType::new(
+        false,
         &::proxmox::api::schema::ObjectSchema::new(
             "A ticket.",
             &[
@@ -107,7 +108,7 @@ fn create_ticket_schema_check() {
             ],
         )
         .schema(),
-    )
+    ))
     .access(Some("Only root can access this."), &Permission::Superuser)
     .protected(true);
     assert_eq!(TEST_METHOD, API_METHOD_CREATE_TICKET);
@@ -184,7 +185,8 @@ fn create_ticket_direct_schema_check() {
             ],
         ),
     )
-    .returns(
+    .returns(::proxmox::api::router::ReturnType::new(
+        false,
         &::proxmox::api::schema::ObjectSchema::new(
             "A ticket.",
             &[
@@ -209,7 +211,7 @@ fn create_ticket_direct_schema_check() {
             ],
         )
         .schema(),
-    )
+    ))
     .access(None, &Permission::World)
     .protected(true);
     assert_eq!(TEST_METHOD, API_METHOD_CREATE_TICKET_DIRECT);
diff --git a/proxmox-api-macro/tests/ext-schema.rs b/proxmox-api-macro/tests/ext-schema.rs
index e9f847d..9fce967 100644
--- a/proxmox-api-macro/tests/ext-schema.rs
+++ b/proxmox-api-macro/tests/ext-schema.rs
@@ -120,7 +120,7 @@ pub fn get_some_text() -> Result<String, Error> {
     returns: {
         properties: {
             "text": {
-                schema: API_RETURN_SCHEMA_GET_SOME_TEXT
+                schema: *API_METHOD_GET_SOME_TEXT.returns.schema
             },
         },
     },
diff --git a/proxmox-api-macro/tests/types.rs b/proxmox-api-macro/tests/types.rs
index e121c3f..80d09ba 100644
--- a/proxmox-api-macro/tests/types.rs
+++ b/proxmox-api-macro/tests/types.rs
@@ -144,7 +144,7 @@ fn selection_test() {
             selection: { type: Selection },
         }
     },
-    returns: { type: Boolean },
+    returns: { optional: true, type: Boolean },
 )]
 /// Check a string.
 ///
@@ -167,7 +167,10 @@ fn string_check_schema_test() {
             ],
         ),
     )
-    .returns(&::proxmox::api::schema::BooleanSchema::new("Whether the string was \"ok\".").schema())
+    .returns(::proxmox::api::router::ReturnType::new(
+        true,
+        &::proxmox::api::schema::BooleanSchema::new("Whether the string was \"ok\".").schema(),
+    ))
     .protected(false);
 
     assert_eq!(TEST_METHOD, API_METHOD_STRING_CHECK);
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 04/18] schema: support AllOf schemas
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (2 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 03/18] api-macro: " Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 05/18] schema: allow AllOf schema as method parameter Wolfgang Bumiller
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/src/api/cli/text_table.rs |  49 ++++++----
 proxmox/src/api/format.rs         |  14 ++-
 proxmox/src/api/schema.rs         | 151 ++++++++++++++++++++++++++++--
 proxmox/src/api/section_config.rs |   2 +-
 4 files changed, 183 insertions(+), 33 deletions(-)

diff --git a/proxmox/src/api/cli/text_table.rs b/proxmox/src/api/cli/text_table.rs
index 7e19ed1..131e667 100644
--- a/proxmox/src/api/cli/text_table.rs
+++ b/proxmox/src/api/cli/text_table.rs
@@ -56,24 +56,25 @@ fn data_to_text(data: &Value, schema: &Schema) -> Result<String, Error> {
             // makes no sense to display Null columns
             bail!("internal error");
         }
-        Schema::Boolean(_boolean_schema) => match data.as_bool() {
+        Schema::Boolean(_) => match data.as_bool() {
             Some(value) => Ok(String::from(if value { "1" } else { "0" })),
             None => bail!("got unexpected data (expected bool)."),
         },
-        Schema::Integer(_integer_schema) => match data.as_i64() {
+        Schema::Integer(_) => match data.as_i64() {
             Some(value) => Ok(format!("{}", value)),
             None => bail!("got unexpected data (expected integer)."),
         },
-        Schema::Number(_number_schema) => match data.as_f64() {
+        Schema::Number(_) => match data.as_f64() {
             Some(value) => Ok(format!("{}", value)),
             None => bail!("got unexpected data (expected number)."),
         },
-        Schema::String(_string_schema) => match data.as_str() {
+        Schema::String(_) => match data.as_str() {
             Some(value) => Ok(value.to_string()),
             None => bail!("got unexpected data (expected string)."),
         },
-        Schema::Object(_object_schema) => Ok(data.to_string()),
-        Schema::Array(_array_schema) => Ok(data.to_string()),
+        Schema::Object(_) => Ok(data.to_string()),
+        Schema::Array(_) => Ok(data.to_string()),
+        Schema::AllOf(_) => Ok(data.to_string()),
     }
 }
 
@@ -325,14 +326,14 @@ struct TableColumn {
     right_align: bool,
 }
 
-fn format_table<W: Write>(
+fn format_table<W: Write, I: Iterator<Item = &'static SchemaPropertyEntry>>(
     output: W,
     list: &mut Vec<Value>,
-    schema: &ObjectSchema,
+    schema: &dyn ObjectSchemaType<PropertyIter = I>,
     options: &TableFormatOptions,
 ) -> Result<(), Error> {
     let properties_to_print = if options.column_config.is_empty() {
-        extract_properties_to_print(schema)
+        extract_properties_to_print(schema.properties())
     } else {
         options
             .column_config
@@ -579,14 +580,14 @@ fn render_table<W: Write>(
     Ok(())
 }
 
-fn format_object<W: Write>(
+fn format_object<W: Write, I: Iterator<Item = &'static SchemaPropertyEntry>>(
     output: W,
     data: &Value,
-    schema: &ObjectSchema,
+    schema: &dyn ObjectSchemaType<PropertyIter = I>,
     options: &TableFormatOptions,
 ) -> Result<(), Error> {
     let properties_to_print = if options.column_config.is_empty() {
-        extract_properties_to_print(schema)
+        extract_properties_to_print(schema.properties())
     } else {
         options
             .column_config
@@ -702,19 +703,23 @@ fn format_object<W: Write>(
     render_table(output, &tabledata, &column_names, options)
 }
 
-fn extract_properties_to_print(schema: &ObjectSchema) -> Vec<String> {
+fn extract_properties_to_print<I>(properties: I) -> Vec<String>
+where
+    I: Iterator<Item = &'static SchemaPropertyEntry>,
+{
     let mut result = Vec::new();
+    let mut opt_properties = Vec::new();
 
-    for (name, optional, _prop_schema) in schema.properties {
-        if !*optional {
-            result.push(name.to_string());
-        }
-    }
-    for (name, optional, _prop_schema) in schema.properties {
+    for (name, optional, _prop_schema) in properties {
         if *optional {
+            opt_properties.push(name.to_string());
+        } else {
             result.push(name.to_string());
         }
     }
+
+    result.extend(opt_properties);
+
     result
 }
 
@@ -759,11 +764,17 @@ pub fn value_to_text<W: Write>(
                 Schema::Object(object_schema) => {
                     format_table(output, list, object_schema, options)?;
                 }
+                Schema::AllOf(all_of_schema) => {
+                    format_table(output, list, all_of_schema, options)?;
+                }
                 _ => {
                     unimplemented!();
                 }
             }
         }
+        Schema::AllOf(all_of_schema) => {
+            format_object(output, data, all_of_schema, options)?;
+        }
     }
     Ok(())
 }
diff --git a/proxmox/src/api/format.rs b/proxmox/src/api/format.rs
index eac2214..719d862 100644
--- a/proxmox/src/api/format.rs
+++ b/proxmox/src/api/format.rs
@@ -96,6 +96,7 @@ pub fn get_schema_type_text(schema: &Schema, _style: ParameterDisplayStyle) -> S
         },
         Schema::Object(_) => String::from("<object>"),
         Schema::Array(_) => String::from("<array>"),
+        Schema::AllOf(_) => String::from("<object>"),
     }
 }
 
@@ -115,6 +116,7 @@ pub fn get_property_description(
         Schema::Integer(ref schema) => (schema.description, schema.default.map(|v| v.to_string())),
         Schema::Number(ref schema) => (schema.description, schema.default.map(|v| v.to_string())),
         Schema::Object(ref schema) => (schema.description, None),
+        Schema::AllOf(ref schema) => (schema.description, None),
         Schema::Array(ref schema) => (schema.description, None),
     };
 
@@ -156,13 +158,16 @@ pub fn get_property_description(
     }
 }
 
-fn dump_api_parameters(param: &ObjectSchema) -> String {
-    let mut res = wrap_text("", "", param.description, 80);
+fn dump_api_parameters<I>(param: &dyn ObjectSchemaType<PropertyIter = I>) -> String
+where
+    I: Iterator<Item = &'static SchemaPropertyEntry>,
+{
+    let mut res = wrap_text("", "", param.description(), 80);
 
     let mut required_list: Vec<String> = Vec::new();
     let mut optional_list: Vec<String> = Vec::new();
 
-    for (prop, optional, schema) in param.properties {
+    for (prop, optional, schema) in param.properties() {
         let param_descr = get_property_description(
             prop,
             &schema,
@@ -237,6 +242,9 @@ fn dump_api_return_schema(returns: &ReturnType) -> String {
         Schema::Object(obj_schema) => {
             res.push_str(&dump_api_parameters(obj_schema));
         }
+        Schema::AllOf(all_of_schema) => {
+            res.push_str(&dump_api_parameters(all_of_schema));
+        }
     }
 
     res.push('\n');
diff --git a/proxmox/src/api/schema.rs b/proxmox/src/api/schema.rs
index d675f8c..f1ceddd 100644
--- a/proxmox/src/api/schema.rs
+++ b/proxmox/src/api/schema.rs
@@ -397,6 +397,13 @@ impl ArraySchema {
     }
 }
 
+/// Property entry in an object schema:
+///
+/// - `name`: The name of the property
+/// - `optional`: Set when the property is optional
+/// - `schema`: Property type schema
+pub type SchemaPropertyEntry = (&'static str, bool, &'static Schema);
+
 /// Lookup table to Schema properties
 ///
 /// Stores a sorted list of `(name, optional, schema)` tuples:
@@ -409,7 +416,7 @@ impl ArraySchema {
 /// a binary search to find items.
 ///
 /// This is a workaround unless RUST can const_fn `Hash::new()`
-pub type SchemaPropertyMap = &'static [(&'static str, bool, &'static Schema)];
+pub type SchemaPropertyMap = &'static [SchemaPropertyEntry];
 
 /// Data type to describe objects (maps).
 #[derive(Debug)]
@@ -462,6 +469,126 @@ impl ObjectSchema {
     }
 }
 
+/// Combines multiple *object* schemas into one.
+///
+/// Note that these are limited to object schemas. Other schemas will produce errors.
+///
+/// Technically this could also contain an `additional_properties` flag, however, in the JSON
+/// Schema[1], this is not supported, so here we simply assume additional properties to be allowed.
+#[derive(Debug)]
+#[cfg_attr(feature = "test-harness", derive(Eq, PartialEq))]
+pub struct AllOfSchema {
+    pub description: &'static str,
+
+    /// The parameter is checked against all of the schemas in the list. Note that all schemas must
+    /// be object schemas.
+    pub list: &'static [&'static Schema],
+}
+
+impl AllOfSchema {
+    pub const fn new(description: &'static str, list: &'static [&'static Schema]) -> Self {
+        Self { description, list }
+    }
+
+    pub const fn schema(self) -> Schema {
+        Schema::AllOf(self)
+    }
+
+    pub fn lookup(&self, key: &str) -> Option<(bool, &Schema)> {
+        for entry in self.list {
+            match entry {
+                Schema::Object(s) => {
+                    if let Some(v) = s.lookup(key) {
+                        return Some(v);
+                    }
+                }
+                _ => panic!("non-object-schema in `AllOfSchema`"),
+            }
+        }
+
+        None
+    }
+}
+
+/// Beside [`ObjectSchema`] we also have an [`AllOfSchema`] which also represents objects.
+pub trait ObjectSchemaType {
+    type PropertyIter: Iterator<Item = &'static SchemaPropertyEntry>;
+
+    fn description(&self) -> &'static str;
+    fn lookup(&self, key: &str) -> Option<(bool, &Schema)>;
+    fn properties(&self) -> Self::PropertyIter;
+    fn additional_properties(&self) -> bool;
+}
+
+impl ObjectSchemaType for ObjectSchema {
+    type PropertyIter = std::slice::Iter<'static, SchemaPropertyEntry>;
+
+    fn description(&self) -> &'static str {
+        self.description
+    }
+
+    fn lookup(&self, key: &str) -> Option<(bool, &Schema)> {
+        ObjectSchema::lookup(self, key)
+    }
+
+    fn properties(&self) -> Self::PropertyIter {
+        self.properties.into_iter()
+    }
+
+    fn additional_properties(&self) -> bool {
+        self.additional_properties
+    }
+}
+
+impl ObjectSchemaType for AllOfSchema {
+    type PropertyIter = AllOfProperties;
+
+    fn description(&self) -> &'static str {
+        self.description
+    }
+
+    fn lookup(&self, key: &str) -> Option<(bool, &Schema)> {
+        AllOfSchema::lookup(self, key)
+    }
+
+    fn properties(&self) -> Self::PropertyIter {
+        AllOfProperties {
+            schemas: self.list.into_iter(),
+            properties: None,
+        }
+    }
+
+    fn additional_properties(&self) -> bool {
+        true
+    }
+}
+
+#[doc(hidden)]
+pub struct AllOfProperties {
+    schemas: std::slice::Iter<'static, &'static Schema>,
+    properties: Option<std::slice::Iter<'static, SchemaPropertyEntry>>,
+}
+
+impl Iterator for AllOfProperties {
+    type Item = &'static SchemaPropertyEntry;
+
+    fn next(&mut self) -> Option<&'static SchemaPropertyEntry> {
+        loop {
+            match self.properties.as_mut().and_then(Iterator::next) {
+                Some(item) => return Some(item),
+                None => match self.schemas.next()? {
+                    Schema::Object(o) => self.properties = Some(o.properties()),
+                    _ => {
+                        // this case is actually illegal
+                        self.properties = None;
+                        continue;
+                    }
+                },
+            }
+        }
+    }
+}
+
 /// Schemas are used to describe complex data types.
 ///
 /// All schema types implement constant builder methods, and a final
@@ -501,6 +628,7 @@ pub enum Schema {
     String(StringSchema),
     Object(ObjectSchema),
     Array(ArraySchema),
+    AllOf(AllOfSchema),
 }
 
 /// A string enum entry. An enum entry must have a value and a description.
@@ -818,21 +946,18 @@ pub fn parse_query_string(
 /// Verify JSON value with `schema`.
 pub fn verify_json(data: &Value, schema: &Schema) -> Result<(), Error> {
     match schema {
-        Schema::Object(object_schema) => {
-            verify_json_object(data, &object_schema)?;
-        }
-        Schema::Array(array_schema) => {
-            verify_json_array(data, &array_schema)?;
-        }
         Schema::Null => {
             if !data.is_null() {
                 bail!("Expected Null, but value is not Null.");
             }
         }
+        Schema::Object(object_schema) => verify_json_object(data, object_schema)?,
+        Schema::Array(array_schema) => verify_json_array(data, &array_schema)?,
         Schema::Boolean(boolean_schema) => verify_json_boolean(data, &boolean_schema)?,
         Schema::Integer(integer_schema) => verify_json_integer(data, &integer_schema)?,
         Schema::Number(number_schema) => verify_json_number(data, &number_schema)?,
         Schema::String(string_schema) => verify_json_string(data, &string_schema)?,
+        Schema::AllOf(all_of_schema) => verify_json_object(data, all_of_schema)?,
     }
     Ok(())
 }
@@ -890,14 +1015,20 @@ pub fn verify_json_array(data: &Value, schema: &ArraySchema) -> Result<(), Error
 }
 
 /// Verify JSON value using an `ObjectSchema`.
-pub fn verify_json_object(data: &Value, schema: &ObjectSchema) -> Result<(), Error> {
+pub fn verify_json_object<I>(
+    data: &Value,
+    schema: &dyn ObjectSchemaType<PropertyIter = I>,
+) -> Result<(), Error>
+where
+    I: Iterator<Item = &'static SchemaPropertyEntry>,
+{
     let map = match data {
         Value::Object(ref map) => map,
         Value::Array(_) => bail!("Expected object - got array."),
         _ => bail!("Expected object - got scalar value."),
     };
 
-    let additional_properties = schema.additional_properties;
+    let additional_properties = schema.additional_properties();
 
     for (key, value) in map {
         if let Some((_optional, prop_schema)) = schema.lookup(&key) {
@@ -917,7 +1048,7 @@ pub fn verify_json_object(data: &Value, schema: &ObjectSchema) -> Result<(), Err
         }
     }
 
-    for (name, optional, _prop_schema) in schema.properties {
+    for (name, optional, _prop_schema) in schema.properties() {
         if !(*optional) && data[name] == Value::Null {
             bail!(
                 "property '{}': property is missing and it is not optional.",
diff --git a/proxmox/src/api/section_config.rs b/proxmox/src/api/section_config.rs
index 30eb784..c15d813 100644
--- a/proxmox/src/api/section_config.rs
+++ b/proxmox/src/api/section_config.rs
@@ -310,7 +310,7 @@ impl SectionConfig {
                 if section_id.chars().any(|c| c.is_control()) {
                     bail!("detected unexpected control character in section ID.");
                 }
-                if let Err(err) = verify_json_object(section_config, &plugin.properties) {
+                if let Err(err) = verify_json_object(section_config, plugin.properties) {
                     bail!("verify section '{}' failed - {}", section_id, err);
                 }
 
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 05/18] schema: allow AllOf schema as method parameter
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (3 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 04/18] schema: support AllOf schemas Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 06/18] api-macro: add 'flatten' to SerdeAttrib Wolfgang Bumiller
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/src/api/cli/command.rs    |  2 +-
 proxmox/src/api/cli/completion.rs | 11 +++--
 proxmox/src/api/cli/format.rs     | 13 +++--
 proxmox/src/api/cli/getopts.rs    | 19 ++++++--
 proxmox/src/api/format.rs         |  2 +-
 proxmox/src/api/router.rs         | 80 +++++++++++++++++++++++++++----
 proxmox/src/api/schema.rs         | 27 +++++++----
 7 files changed, 122 insertions(+), 32 deletions(-)

diff --git a/proxmox/src/api/cli/command.rs b/proxmox/src/api/cli/command.rs
index 50f5979..fa447ba 100644
--- a/proxmox/src/api/cli/command.rs
+++ b/proxmox/src/api/cli/command.rs
@@ -32,7 +32,7 @@ fn parse_arguments(prefix: &str, cli_cmd: &CliCommand, args: Vec<String>) -> Res
         &args,
         cli_cmd.arg_param,
         &cli_cmd.fixed_param,
-        &cli_cmd.info.parameters,
+        cli_cmd.info.parameters,
     ) {
         Ok((p, r)) => (p, r),
         Err(err) => {
diff --git a/proxmox/src/api/cli/completion.rs b/proxmox/src/api/cli/completion.rs
index 2bb8197..42b3915 100644
--- a/proxmox/src/api/cli/completion.rs
+++ b/proxmox/src/api/cli/completion.rs
@@ -1,10 +1,11 @@
 use super::*;
 
+use crate::api::router::ParameterSchema;
 use crate::api::schema::*;
 
 fn record_done_argument(
     done: &mut HashMap<String, String>,
-    parameters: &ObjectSchema,
+    parameters: ParameterSchema,
     key: &str,
     value: &str,
 ) {
@@ -119,11 +120,11 @@ fn get_simple_completion(
         let mut errors = ParameterError::new(); // we simply ignore any parsing errors here
         let (data, _remaining) = getopts::parse_argument_list(
             &args[0..args.len() - 1],
-            &cli_cmd.info.parameters,
+            cli_cmd.info.parameters,
             &mut errors,
         );
         for (key, value) in &data {
-            record_done_argument(done, &cli_cmd.info.parameters, key, value);
+            record_done_argument(done, cli_cmd.info.parameters, key, value);
         }
     }
 
@@ -148,7 +149,7 @@ fn get_simple_completion(
     }
 
     let mut completions = Vec::new();
-    for (name, _optional, _schema) in cli_cmd.info.parameters.properties {
+    for (name, _optional, _schema) in cli_cmd.info.parameters.properties() {
         if done.contains_key(*name) {
             continue;
         }
@@ -210,7 +211,7 @@ fn get_nested_completion(def: &CommandLineInterface, args: &[String]) -> Vec<Str
         CommandLineInterface::Simple(cli_cmd) => {
             let mut done: HashMap<String, String> = HashMap::new();
             cli_cmd.fixed_param.iter().for_each(|(key, value)| {
-                record_done_argument(&mut done, &cli_cmd.info.parameters, &key, &value);
+                record_done_argument(&mut done, cli_cmd.info.parameters, &key, &value);
             });
             get_simple_completion(cli_cmd, &mut done, &cli_cmd.arg_param, args)
         }
diff --git a/proxmox/src/api/cli/format.rs b/proxmox/src/api/cli/format.rs
index a4e7c02..f780b1a 100644
--- a/proxmox/src/api/cli/format.rs
+++ b/proxmox/src/api/cli/format.rs
@@ -110,7 +110,7 @@ pub fn generate_usage_str(
 
     let mut options = String::new();
 
-    for (prop, optional, param_schema) in schema.properties {
+    for (prop, optional, param_schema) in schema.properties() {
         if done_hash.contains(prop) {
             continue;
         }
@@ -150,11 +150,18 @@ pub fn generate_usage_str(
         DocumentationFormat::Long => format!("{}{}{}{}\n", indent, prefix, args, option_indicator),
         DocumentationFormat::Full => format!(
             "{}{}{}{}\n\n{}\n\n",
-            indent, prefix, args, option_indicator, schema.description
+            indent,
+            prefix,
+            args,
+            option_indicator,
+            schema.description()
         ),
         DocumentationFormat::ReST => format!(
             "``{}{}{}``\n\n{}\n\n",
-            prefix, args, option_indicator, schema.description
+            prefix,
+            args,
+            option_indicator,
+            schema.description()
         ),
     };
 
diff --git a/proxmox/src/api/cli/getopts.rs b/proxmox/src/api/cli/getopts.rs
index 6fd48e8..adf0658 100644
--- a/proxmox/src/api/cli/getopts.rs
+++ b/proxmox/src/api/cli/getopts.rs
@@ -3,6 +3,7 @@ use std::collections::HashMap;
 use anyhow::*;
 use serde_json::Value;
 
+use crate::api::router::ParameterSchema;
 use crate::api::schema::*;
 
 #[derive(Debug)]
@@ -57,7 +58,7 @@ fn parse_argument(arg: &str) -> RawArgument {
 /// Returns parsed data and the remaining arguments as two separate array
 pub(crate) fn parse_argument_list<T: AsRef<str>>(
     args: &[T],
-    schema: &ObjectSchema,
+    schema: ParameterSchema,
     errors: &mut ParameterError,
 ) -> (Vec<(String, String)>, Vec<String>) {
     let mut data: Vec<(String, String)> = vec![];
@@ -149,7 +150,7 @@ pub fn parse_arguments<T: AsRef<str>>(
     args: &[T],
     arg_param: &[&str],
     fixed_param: &HashMap<&'static str, String>,
-    schema: &ObjectSchema,
+    schema: ParameterSchema,
 ) -> Result<(Value, Vec<String>), ParameterError> {
     let mut errors = ParameterError::new();
 
@@ -229,7 +230,12 @@ fn test_boolean_arg() {
     variants.push((vec!["--enable", "false"], false));
 
     for (args, expect) in variants {
-        let res = parse_arguments(&args, &vec![], &HashMap::new(), &PARAMETERS);
+        let res = parse_arguments(
+            &args,
+            &vec![],
+            &HashMap::new(),
+            ParameterSchema::from(&PARAMETERS),
+        );
         assert!(res.is_ok());
         if let Ok((options, remaining)) = res {
             assert!(options["enable"] == expect);
@@ -249,7 +255,12 @@ fn test_argument_paramenter() {
     );
 
     let args = vec!["-enable", "local"];
-    let res = parse_arguments(&args, &vec!["storage"], &HashMap::new(), &PARAMETERS);
+    let res = parse_arguments(
+        &args,
+        &vec!["storage"],
+        &HashMap::new(),
+        ParameterSchema::from(&PARAMETERS),
+    );
     assert!(res.is_ok());
     if let Ok((options, remaining)) = res {
         assert!(options["enable"] == true);
diff --git a/proxmox/src/api/format.rs b/proxmox/src/api/format.rs
index 719d862..2ce9597 100644
--- a/proxmox/src/api/format.rs
+++ b/proxmox/src/api/format.rs
@@ -256,7 +256,7 @@ fn dump_method_definition(method: &str, path: &str, def: Option<&ApiMethod>) ->
     match def {
         None => None,
         Some(api_method) => {
-            let param_descr = dump_api_parameters(api_method.parameters);
+            let param_descr = dump_api_parameters(&api_method.parameters);
 
             let return_descr = dump_api_return_schema(&api_method.returns);
 
diff --git a/proxmox/src/api/router.rs b/proxmox/src/api/router.rs
index 9fb9ec1..2f4b6c4 100644
--- a/proxmox/src/api/router.rs
+++ b/proxmox/src/api/router.rs
@@ -10,7 +10,7 @@ use hyper::Body;
 use percent_encoding::percent_decode_str;
 use serde_json::Value;
 
-use crate::api::schema::{self, ObjectSchema, Schema};
+use crate::api::schema::{self, AllOfSchema, ObjectSchema, Schema};
 use crate::api::RpcEnvironment;
 
 use super::Permission;
@@ -36,10 +36,11 @@ use super::Permission;
 ///    &ObjectSchema::new("Hello World Example", &[])
 /// );
 /// ```
-pub type ApiHandlerFn = &'static (dyn Fn(Value, &ApiMethod, &mut dyn RpcEnvironment) -> Result<Value, Error>
-              + Send
-              + Sync
-              + 'static);
+pub type ApiHandlerFn =
+    &'static (dyn Fn(Value, &ApiMethod, &mut dyn RpcEnvironment) -> Result<Value, Error>
+                  + Send
+                  + Sync
+                  + 'static);
 
 /// Asynchronous API handlers
 ///
@@ -67,7 +68,11 @@ pub type ApiHandlerFn = &'static (dyn Fn(Value, &ApiMethod, &mut dyn RpcEnvironm
 ///    &ObjectSchema::new("Hello World Example (async)", &[])
 /// );
 /// ```
-pub type ApiAsyncHandlerFn = &'static (dyn for<'a> Fn(Value, &'static ApiMethod, &'a mut dyn RpcEnvironment) -> ApiFuture<'a>
+pub type ApiAsyncHandlerFn = &'static (dyn for<'a> Fn(
+    Value,
+    &'static ApiMethod,
+    &'a mut dyn RpcEnvironment,
+) -> ApiFuture<'a>
               + Send
               + Sync);
 
@@ -425,6 +430,59 @@ impl ReturnType {
     }
 }
 
+/// Parameters are objects, but we have two types of object schemas, the regular one and the
+/// `AllOf` schema.
+#[derive(Clone, Copy, Debug)]
+#[cfg_attr(feature = "test-harness", derive(Eq, PartialEq))]
+pub enum ParameterSchema {
+    Object(&'static ObjectSchema),
+    AllOf(&'static AllOfSchema),
+}
+
+impl schema::ObjectSchemaType for ParameterSchema {
+    type PropertyIter = Box<dyn Iterator<Item = &'static schema::SchemaPropertyEntry>>;
+
+    fn description(&self) -> &'static str {
+        match self {
+            ParameterSchema::Object(o) => o.description(),
+            ParameterSchema::AllOf(o) => o.description(),
+        }
+    }
+
+    fn lookup(&self, key: &str) -> Option<(bool, &Schema)> {
+        match self {
+            ParameterSchema::Object(o) => o.lookup(key),
+            ParameterSchema::AllOf(o) => o.lookup(key),
+        }
+    }
+
+    fn properties(&self) -> Self::PropertyIter {
+        match self {
+            ParameterSchema::Object(o) => Box::new(o.properties()),
+            ParameterSchema::AllOf(o) => Box::new(o.properties()),
+        }
+    }
+
+    fn additional_properties(&self) -> bool {
+        match self {
+            ParameterSchema::Object(o) => o.additional_properties(),
+            ParameterSchema::AllOf(o) => o.additional_properties(),
+        }
+    }
+}
+
+impl From<&'static ObjectSchema> for ParameterSchema {
+    fn from(schema: &'static ObjectSchema) -> Self {
+        ParameterSchema::Object(schema)
+    }
+}
+
+impl From<&'static AllOfSchema> for ParameterSchema {
+    fn from(schema: &'static AllOfSchema) -> Self {
+        ParameterSchema::AllOf(schema)
+    }
+}
+
 /// This struct defines a synchronous API call which returns the result as json `Value`
 #[cfg_attr(feature = "test-harness", derive(Eq, PartialEq))]
 pub struct ApiMethod {
@@ -435,7 +493,7 @@ pub struct ApiMethod {
     /// should do a tzset afterwards
     pub reload_timezone: bool,
     /// Parameter type Schema
-    pub parameters: &'static schema::ObjectSchema,
+    pub parameters: ParameterSchema,
     /// Return type Schema
     pub returns: ReturnType,
     /// Handler function
@@ -456,7 +514,7 @@ impl std::fmt::Debug for ApiMethod {
 }
 
 impl ApiMethod {
-    pub const fn new(handler: &'static ApiHandler, parameters: &'static ObjectSchema) -> Self {
+    pub const fn new_full(handler: &'static ApiHandler, parameters: ParameterSchema) -> Self {
         Self {
             parameters,
             handler,
@@ -470,9 +528,13 @@ impl ApiMethod {
         }
     }
 
+    pub const fn new(handler: &'static ApiHandler, parameters: &'static ObjectSchema) -> Self {
+        Self::new_full(handler, ParameterSchema::Object(parameters))
+    }
+
     pub const fn new_dummy(parameters: &'static ObjectSchema) -> Self {
         Self {
-            parameters,
+            parameters: ParameterSchema::Object(parameters),
             handler: &DUMMY_HANDLER,
             returns: ReturnType::new(false, &NULL_SCHEMA),
             protected: false,
diff --git a/proxmox/src/api/schema.rs b/proxmox/src/api/schema.rs
index f1ceddd..1378d78 100644
--- a/proxmox/src/api/schema.rs
+++ b/proxmox/src/api/schema.rs
@@ -10,6 +10,7 @@ use anyhow::{bail, format_err, Error};
 use serde_json::{json, Value};
 use url::form_urlencoded;
 
+use super::router::ParameterSchema;
 use crate::api::const_regex::ConstRegexPattern;
 
 /// Error type for schema validation
@@ -764,7 +765,7 @@ pub fn parse_boolean(value_str: &str) -> Result<bool, Error> {
 }
 
 /// Parse a complex property string (`ApiStringFormat::PropertyString`)
-pub fn parse_property_string(value_str: &str, schema: &Schema) -> Result<Value, Error> {
+pub fn parse_property_string(value_str: &str, schema: &'static Schema) -> Result<Value, Error> {
     match schema {
         Schema::Object(object_schema) => {
             let mut param_list: Vec<(String, String)> = vec![];
@@ -783,7 +784,7 @@ pub fn parse_property_string(value_str: &str, schema: &Schema) -> Result<Value,
                 }
             }
 
-            parse_parameter_strings(&param_list, &object_schema, true).map_err(Error::from)
+            parse_parameter_strings(&param_list, object_schema, true).map_err(Error::from)
         }
         Schema::Array(array_schema) => {
             let mut array: Vec<Value> = vec![];
@@ -839,16 +840,24 @@ pub fn parse_simple_value(value_str: &str, schema: &Schema) -> Result<Value, Err
 ///
 /// - `test_required`: is set, checks if all required properties are
 ///   present.
-pub fn parse_parameter_strings(
+pub fn parse_parameter_strings<T: Into<ParameterSchema>>(
     data: &[(String, String)],
-    schema: &ObjectSchema,
+    schema: T,
+    test_required: bool,
+) -> Result<Value, ParameterError> {
+    do_parse_parameter_strings(data, schema.into(), test_required)
+}
+
+fn do_parse_parameter_strings(
+    data: &[(String, String)],
+    schema: ParameterSchema,
     test_required: bool,
 ) -> Result<Value, ParameterError> {
     let mut params = json!({});
 
     let mut errors = ParameterError::new();
 
-    let additional_properties = schema.additional_properties;
+    let additional_properties = schema.additional_properties();
 
     for (key, value) in data {
         if let Some((_optional, prop_schema)) = schema.lookup(&key) {
@@ -911,7 +920,7 @@ pub fn parse_parameter_strings(
     }
 
     if test_required && errors.is_empty() {
-        for (name, optional, _prop_schema) in schema.properties {
+        for (name, optional, _prop_schema) in schema.properties() {
             if !(*optional) && params[name] == Value::Null {
                 errors.push(format_err!(
                     "parameter '{}': parameter is missing and it is not optional.",
@@ -931,16 +940,16 @@ pub fn parse_parameter_strings(
 /// Parse a `form_urlencoded` query string and verify with object schema
 /// - `test_required`: is set, checks if all required properties are
 ///   present.
-pub fn parse_query_string(
+pub fn parse_query_string<T: Into<ParameterSchema>>(
     query: &str,
-    schema: &ObjectSchema,
+    schema: T,
     test_required: bool,
 ) -> Result<Value, ParameterError> {
     let param_list: Vec<(String, String)> = form_urlencoded::parse(query.as_bytes())
         .into_owned()
         .collect();
 
-    parse_parameter_strings(&param_list, schema, test_required)
+    parse_parameter_strings(&param_list, schema.into(), test_required)
 }
 
 /// Verify JSON value with `schema`.
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 06/18] api-macro: add 'flatten' to SerdeAttrib
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (4 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 05/18] schema: allow AllOf schema as method parameter Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 07/18] api-macro: forbid flattened fields Wolfgang Bumiller
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/serde.rs | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/proxmox-api-macro/src/serde.rs b/proxmox-api-macro/src/serde.rs
index a08f461..2829030 100644
--- a/proxmox-api-macro/src/serde.rs
+++ b/proxmox-api-macro/src/serde.rs
@@ -159,12 +159,15 @@ impl TryFrom<&[syn::Attribute]> for ContainerAttrib {
 #[derive(Default)]
 pub struct SerdeAttrib {
     pub rename: Option<FieldName>,
+    pub flatten: bool,
 }
 
 impl TryFrom<&[syn::Attribute]> for SerdeAttrib {
     type Error = syn::Error;
 
     fn try_from(attributes: &[syn::Attribute]) -> Result<Self, syn::Error> {
+        use syn::{Meta, NestedMeta};
+
         let mut this: Self = Default::default();
 
         for attrib in attributes {
@@ -174,8 +177,8 @@ impl TryFrom<&[syn::Attribute]> for SerdeAttrib {
 
             let args: AttrArgs = syn::parse2(attrib.tokens.clone())?;
             for arg in args.args {
-                if let syn::NestedMeta::Meta(syn::Meta::NameValue(var)) = arg {
-                    if var.path.is_ident("rename") {
+                match arg {
+                    NestedMeta::Meta(Meta::NameValue(var)) if var.path.is_ident("rename") => {
                         match var.lit {
                             syn::Lit::Str(lit) => {
                                 let rename = FieldName::from(&lit);
@@ -187,6 +190,10 @@ impl TryFrom<&[syn::Attribute]> for SerdeAttrib {
                             _ => error!(var.lit => "'rename' value must be a string literal"),
                         }
                     }
+                    NestedMeta::Meta(Meta::Path(path)) if path.is_ident("flatten") => {
+                        this.flatten = true;
+                    }
+                    _ => continue,
                 }
             }
         }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 07/18] api-macro: forbid flattened fields
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (5 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 06/18] api-macro: add 'flatten' to SerdeAttrib Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 08/18] api-macro: add more standard Maybe methods Wolfgang Bumiller
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

They don't appear in the json data structure and therefore
should not be named separately in the schema. Structs with
flattened fields will become an `AllOf` schema instead.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/structs.rs | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs
index 71cdc8a..a101308 100644
--- a/proxmox-api-macro/src/api/structs.rs
+++ b/proxmox-api-macro/src/api/structs.rs
@@ -162,8 +162,20 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
                 }
             };
 
+            if attrs.flatten {
+                if let Some(field) = schema_fields.remove(&name) {
+                    error!(
+                        field.0.span(),
+                        "flattened field should not appear in schema, \
+                         its name does not appear in serialized data",
+                    );
+                }
+            }
+
             match schema_fields.remove(&name) {
-                Some(field_def) => handle_regular_field(field_def, field, false)?,
+                Some(field_def) => {
+                    handle_regular_field(field_def, field, false)?;
+                }
                 None => {
                     let mut field_def = (
                         FieldName::new(name.clone(), span),
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 08/18] api-macro: add more standard Maybe methods
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (6 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 07/18] api-macro: forbid flattened fields Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 09/18] api-macro: suport AllOf on structs Wolfgang Bumiller
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Note that any methods added there should be oriented around
`Option`.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/util.rs | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/proxmox-api-macro/src/util.rs b/proxmox-api-macro/src/util.rs
index e74e04b..87e6414 100644
--- a/proxmox-api-macro/src/util.rs
+++ b/proxmox-api-macro/src/util.rs
@@ -584,6 +584,12 @@ pub enum Maybe<T> {
     None,
 }
 
+impl<T> Default for Maybe<T> {
+    fn default() -> Self {
+        Maybe::None
+    }
+}
+
 impl<T> Maybe<T> {
     pub fn as_ref(&self) -> Maybe<&T> {
         match self {
@@ -600,6 +606,13 @@ impl<T> Maybe<T> {
         }
     }
 
+    pub fn ok(self) -> Option<T> {
+        match self {
+            Maybe::Explicit(v) | Maybe::Derived(v) => Some(v),
+            Maybe::None => None,
+        }
+    }
+
     pub fn ok_or_else<E, F>(self, other: F) -> Result<T, E>
     where
         F: FnOnce() -> E,
@@ -613,6 +626,14 @@ impl<T> Maybe<T> {
     pub fn is_none(&self) -> bool {
         matches!(self, Maybe::None)
     }
+
+    pub fn is_explicit(&self) -> bool {
+        matches!(self, Maybe::Explicit(_))
+    }
+
+    pub fn take(&mut self) -> Self {
+        std::mem::take(self)
+    }
 }
 
 impl<T> Into<Option<T>> for Maybe<T> {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 09/18] api-macro: suport AllOf on structs
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (7 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 08/18] api-macro: add more standard Maybe methods Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 10/18] schema: ExtractValueDeserializer Wolfgang Bumiller
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/mod.rs     |  19 +++++
 proxmox-api-macro/src/api/structs.rs | 123 ++++++++++++++++++++++++---
 proxmox-api-macro/tests/allof.rs     | 118 +++++++++++++++++++++++++
 3 files changed, 246 insertions(+), 14 deletions(-)
 create mode 100644 proxmox-api-macro/tests/allof.rs

diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs
index 815e167..3cf1309 100644
--- a/proxmox-api-macro/src/api/mod.rs
+++ b/proxmox-api-macro/src/api/mod.rs
@@ -397,6 +397,11 @@ impl SchemaObject {
         }
     }
 
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.properties_.is_empty()
+    }
+
     #[inline]
     fn properties_mut(&mut self) -> &mut [(FieldName, bool, Schema)] {
         &mut self.properties_
@@ -458,6 +463,20 @@ impl SchemaObject {
             .find(|p| p.0.as_ident_str() == key)
     }
 
+    fn remove_property_by_ident(&mut self, key: &str) -> bool {
+        match self
+            .properties_
+            .iter()
+            .position(|(name, _, _)| name.as_ident_str() == key)
+        {
+            Some(index) => {
+                self.properties_.remove(index);
+                true
+            }
+            None => false,
+        }
+    }
+
     fn extend_properties(&mut self, new_fields: Vec<(FieldName, bool, Schema)>) {
         self.properties_.extend(new_fields);
         self.sort_properties();
diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs
index a101308..db6a290 100644
--- a/proxmox-api-macro/src/api/structs.rs
+++ b/proxmox-api-macro/src/api/structs.rs
@@ -21,7 +21,7 @@ use quote::quote_spanned;
 use super::Schema;
 use crate::api::{self, SchemaItem};
 use crate::serde;
-use crate::util::{self, FieldName, JSONObject};
+use crate::util::{self, FieldName, JSONObject, Maybe};
 
 pub fn handle_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<TokenStream, Error> {
     match &stru.fields {
@@ -142,6 +142,9 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
 
     let container_attrs = serde::ContainerAttrib::try_from(&stru.attrs[..])?;
 
+    let mut all_of_schemas = TokenStream::new();
+    let mut to_remove = Vec::new();
+
     if let syn::Fields::Named(ref fields) = &stru.fields {
         for field in &fields.named {
             let attrs = serde::SerdeAttrib::try_from(&field.attrs[..])?;
@@ -162,19 +165,34 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
                 }
             };
 
-            if attrs.flatten {
-                if let Some(field) = schema_fields.remove(&name) {
-                    error!(
-                        field.0.span(),
-                        "flattened field should not appear in schema, \
-                         its name does not appear in serialized data",
-                    );
-                }
-            }
-
             match schema_fields.remove(&name) {
                 Some(field_def) => {
+                    if attrs.flatten {
+                        to_remove.push(name.clone());
+
+                        let name = &field_def.0;
+                        let optional = &field_def.1;
+                        let schema = &field_def.2;
+                        if schema.description.is_explicit() {
+                            error!(
+                                name.span(),
+                                "flattened field should not have a description, \
+                                 it does not appear in serialized data as a field",
+                            );
+                        }
+
+                        if *optional {
+                            error!(name.span(), "optional flattened fields are not supported");
+                        }
+                    }
+
                     handle_regular_field(field_def, field, false)?;
+
+                    if attrs.flatten {
+                        all_of_schemas.extend(quote::quote! {&});
+                        field_def.2.to_schema(&mut all_of_schemas)?;
+                        all_of_schemas.extend(quote::quote! {,});
+                    }
                 }
                 None => {
                     let mut field_def = (
@@ -183,7 +201,15 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
                         Schema::blank(span),
                     );
                     handle_regular_field(&mut field_def, field, true)?;
-                    new_fields.push(field_def);
+
+                    if attrs.flatten {
+                        all_of_schemas.extend(quote::quote! {&});
+                        field_def.2.to_schema(&mut all_of_schemas)?;
+                        all_of_schemas.extend(quote::quote! {,});
+                        to_remove.push(name.clone());
+                    } else {
+                        new_fields.push(field_def);
+                    }
                 }
             }
         }
@@ -200,14 +226,83 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
         );
     }
 
-    // add the fields we derived:
     if let api::SchemaItem::Object(ref mut obj) = &mut schema.item {
+        // remove flattened fields
+        for field in to_remove {
+            if !obj.remove_property_by_ident(&field) {
+                error!(
+                    schema.span,
+                    "internal error: failed to remove property {:?} from object schema", field,
+                );
+            }
+        }
+
+        // add derived fields
         obj.extend_properties(new_fields);
     } else {
         panic!("handle_regular_struct with non-object schema");
     }
 
-    finish_schema(schema, &stru, &stru.ident)
+    if all_of_schemas.is_empty() {
+        finish_schema(schema, &stru, &stru.ident)
+    } else {
+        let name = &stru.ident;
+
+        // take out the inner object schema's description
+        let description = match schema.description.take().ok() {
+            Some(description) => description,
+            None => {
+                error!(schema.span, "missing description on api type struct");
+                syn::LitStr::new("<missing description>", schema.span)
+            }
+        };
+        // and replace it with a "dummy"
+        schema.description = Maybe::Derived(syn::LitStr::new(
+            &format!("<INNER: {}>", description.value()),
+            description.span(),
+        ));
+
+        // now check if it even has any fields
+        let has_fields = match &schema.item {
+            api::SchemaItem::Object(obj) => !obj.is_empty(),
+            _ => panic!("object schema is not an object schema?"),
+        };
+
+        let (inner_schema, inner_schema_ref) = if has_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();
+                schema.to_schema(&mut ts)?;
+                ts
+            };
+
+            (
+                quote_spanned!(name.span() =>
+                    const INNER_API_SCHEMA: ::proxmox::api::schema::Schema = #obj_schema;
+                ),
+                quote_spanned!(name.span() => &Self::INNER_API_SCHEMA,),
+            )
+        } else {
+            // otherwise it stays empty
+            (TokenStream::new(), TokenStream::new())
+        };
+
+        Ok(quote_spanned!(name.span() =>
+            #stru
+            impl #name {
+                #inner_schema
+                pub const API_SCHEMA: ::proxmox::api::schema::Schema =
+                    ::proxmox::api::schema::AllOfSchema::new(
+                        #description,
+                        &[
+                            #inner_schema_ref
+                            #all_of_schemas
+                        ],
+                    )
+                    .schema();
+            }
+        ))
+    }
 }
 
 /// Field handling:
diff --git a/proxmox-api-macro/tests/allof.rs b/proxmox-api-macro/tests/allof.rs
new file mode 100644
index 0000000..56e86d7
--- /dev/null
+++ b/proxmox-api-macro/tests/allof.rs
@@ -0,0 +1,118 @@
+//! Testing the `AllOf` schema on structs and methods.
+
+use proxmox::api::schema;
+use proxmox_api_macro::api;
+
+use serde::{Deserialize, Serialize};
+
+pub const NAME_SCHEMA: schema::Schema = schema::StringSchema::new("Name.").schema();
+pub const VALUE_SCHEMA: schema::Schema = schema::IntegerSchema::new("Value.").schema();
+pub const INDEX_SCHEMA: schema::Schema = schema::IntegerSchema::new("Index.").schema();
+pub const TEXT_SCHEMA: schema::Schema = schema::StringSchema::new("Text.").schema();
+
+#[api(
+    properties: {
+        name: { schema: NAME_SCHEMA },
+        value: { schema: VALUE_SCHEMA },
+    }
+)]
+/// Name and value.
+#[derive(Deserialize, Serialize)]
+struct NameValue {
+    name: String,
+    value: u64,
+}
+
+#[api(
+    properties: {
+        index: { schema: INDEX_SCHEMA },
+        text: { schema: TEXT_SCHEMA },
+    }
+)]
+/// Index and text.
+#[derive(Deserialize, Serialize)]
+struct IndexText {
+    index: u64,
+    text: String,
+}
+
+#[api(
+    properties: {
+        nv: { type: NameValue },
+        it: { type: IndexText },
+    },
+)]
+/// Name, value, index and text.
+#[derive(Deserialize, Serialize)]
+struct Nvit {
+    #[serde(flatten)]
+    nv: NameValue,
+
+    #[serde(flatten)]
+    it: IndexText,
+}
+
+#[test]
+fn test_nvit() {
+    const TEST_NAME_VALUE_SCHEMA: ::proxmox::api::schema::Schema =
+        ::proxmox::api::schema::ObjectSchema::new(
+            "Name and value.",
+            &[
+                ("name", false, &NAME_SCHEMA),
+                ("value", false, &VALUE_SCHEMA),
+            ],
+        )
+        .schema();
+
+    const TEST_SCHEMA: ::proxmox::api::schema::Schema = ::proxmox::api::schema::AllOfSchema::new(
+        "Name, value, index and text.",
+        &[&TEST_NAME_VALUE_SCHEMA, &IndexText::API_SCHEMA],
+    )
+    .schema();
+
+    assert_eq!(TEST_SCHEMA, Nvit::API_SCHEMA);
+}
+
+#[api(
+    properties: {
+        nv: { type: NameValue },
+        it: { type: IndexText },
+    },
+)]
+/// Extra Schema
+#[derive(Deserialize, Serialize)]
+struct WithExtra {
+    #[serde(flatten)]
+    nv: NameValue,
+
+    #[serde(flatten)]
+    it: IndexText,
+
+    /// Extra field.
+    extra: String,
+}
+
+#[test]
+fn test_extra() {
+    const INNER_SCHEMA: ::proxmox::api::schema::Schema = ::proxmox::api::schema::ObjectSchema::new(
+        "<INNER: Extra Schema>",
+        &[(
+            "extra",
+            false,
+            &::proxmox::api::schema::StringSchema::new("Extra field.").schema(),
+        )],
+    )
+    .schema();
+
+    const TEST_SCHEMA: ::proxmox::api::schema::Schema = ::proxmox::api::schema::AllOfSchema::new(
+        "Extra Schema",
+        &[
+            &INNER_SCHEMA,
+            &NameValue::API_SCHEMA,
+            &IndexText::API_SCHEMA,
+        ],
+    )
+    .schema();
+
+    assert_eq!(TEST_SCHEMA, WithExtra::API_SCHEMA);
+}
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 10/18] schema: ExtractValueDeserializer
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (8 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 09/18] api-macro: suport AllOf on structs Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 11/18] api-macro: object schema entry tuple -> struct Wolfgang Bumiller
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

A deserializer which takes an `&mut Value` and an object
schema reference and deserializes by extracting (removing)
the values from the references serde Value.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/src/api/de.rs  | 270 +++++++++++++++++++++++++++++++++++++++++
 proxmox/src/api/mod.rs |   2 +
 2 files changed, 272 insertions(+)
 create mode 100644 proxmox/src/api/de.rs

diff --git a/proxmox/src/api/de.rs b/proxmox/src/api/de.rs
new file mode 100644
index 0000000..b48fd85
--- /dev/null
+++ b/proxmox/src/api/de.rs
@@ -0,0 +1,270 @@
+//! Partial object deserialization by extracting object portions from a Value using an api schema.
+
+use std::fmt;
+
+use serde::de::{self, IntoDeserializer, Visitor};
+use serde_json::Value;
+
+use crate::api::schema::{ObjectSchema, ObjectSchemaType, Schema};
+
+pub struct Error {
+    inner: anyhow::Error,
+}
+
+impl fmt::Debug for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Debug::fmt(&self.inner, f)
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fmt::Display::fmt(&self.inner, f)
+    }
+}
+
+impl std::error::Error for Error {}
+
+impl serde::de::Error for Error {
+    fn custom<T: fmt::Display>(msg: T) -> Self {
+        Self {
+            inner: anyhow::format_err!("{}", msg),
+        }
+    }
+}
+
+impl From<serde_json::Error> for Error {
+    fn from(inner: serde_json::Error) -> Self {
+        Error {
+            inner: inner.into(),
+        }
+    }
+}
+
+pub struct ExtractValueDeserializer<'o> {
+    object: &'o mut serde_json::Map<String, Value>,
+    schema: &'static ObjectSchema,
+}
+
+impl<'o> ExtractValueDeserializer<'o> {
+    pub fn try_new(
+        object: &'o mut serde_json::Map<String, Value>,
+        schema: &'static Schema,
+    ) -> Option<Self> {
+        match schema {
+            Schema::Object(schema) => Some(Self { object, schema }),
+            _ => None,
+        }
+    }
+}
+
+macro_rules! deserialize_non_object {
+    ($name:ident) => {
+        fn $name<V>(self, _visitor: V) -> Result<V::Value, Self::Error>
+        where
+            V: Visitor<'de>,
+        {
+            Err(de::Error::custom(
+                "deserializing partial object into type which is not an object",
+            ))
+        }
+    };
+    ($name:ident ( $($args:tt)* )) => {
+        fn $name<V>(self, $($args)*, _visitor: V) -> Result<V::Value, Self::Error>
+        where
+            V: Visitor<'de>,
+        {
+            Err(de::Error::custom(
+                "deserializing partial object into type which is not an object",
+            ))
+        }
+    };
+}
+
+impl<'de> de::Deserializer<'de> for ExtractValueDeserializer<'de> {
+    type Error = Error;
+
+    fn deserialize_any<V>(self, visitor: V) -> Result<V::Value, Error>
+    where
+        V: Visitor<'de>,
+    {
+        self.deserialize_map(visitor)
+    }
+
+    fn deserialize_map<V>(self, visitor: V) -> Result<V::Value, Error>
+    where
+        V: Visitor<'de>,
+    {
+        visitor.visit_map(MapAccess::<'de>::new(
+            self.object,
+            self.schema.properties().map(|(name, _, _)| *name),
+        ))
+    }
+
+    fn deserialize_struct<V>(
+        self,
+        _name: &'static str,
+        _fields: &'static [&'static str],
+        visitor: V,
+    ) -> Result<V::Value, Error>
+    where
+        V: Visitor<'de>,
+    {
+        visitor.visit_map(MapAccess::<'de>::new(
+            self.object,
+            self.schema.properties().map(|(name, _, _)| *name),
+        ))
+    }
+
+    deserialize_non_object!(deserialize_i8);
+    deserialize_non_object!(deserialize_i16);
+    deserialize_non_object!(deserialize_i32);
+    deserialize_non_object!(deserialize_i64);
+    deserialize_non_object!(deserialize_u8);
+    deserialize_non_object!(deserialize_u16);
+    deserialize_non_object!(deserialize_u32);
+    deserialize_non_object!(deserialize_u64);
+    deserialize_non_object!(deserialize_f32);
+    deserialize_non_object!(deserialize_f64);
+    deserialize_non_object!(deserialize_char);
+    deserialize_non_object!(deserialize_bool);
+    deserialize_non_object!(deserialize_str);
+    deserialize_non_object!(deserialize_string);
+    deserialize_non_object!(deserialize_bytes);
+    deserialize_non_object!(deserialize_byte_buf);
+    deserialize_non_object!(deserialize_option);
+    deserialize_non_object!(deserialize_seq);
+    deserialize_non_object!(deserialize_unit);
+    deserialize_non_object!(deserialize_identifier);
+    deserialize_non_object!(deserialize_unit_struct(_: &'static str));
+    deserialize_non_object!(deserialize_newtype_struct(_: &'static str));
+    deserialize_non_object!(deserialize_tuple(_: usize));
+    deserialize_non_object!(deserialize_tuple_struct(_: &'static str, _: usize));
+    deserialize_non_object!(deserialize_enum(
+        _: &'static str,
+        _: &'static [&'static str]
+    ));
+
+    fn deserialize_ignored_any<V>(self, visitor: V) -> Result<V::Value, Self::Error>
+    where
+        V: Visitor<'de>,
+    {
+        visitor.visit_unit()
+    }
+}
+
+struct MapAccess<'o, I> {
+    object: &'o mut serde_json::Map<String, Value>,
+    iter: I,
+    value: Option<Value>,
+}
+
+impl<'o, I> MapAccess<'o, I>
+where
+    I: Iterator<Item = &'static str>,
+{
+    fn new(object: &'o mut serde_json::Map<String, Value>, iter: I) -> Self {
+        Self {
+            object,
+            iter,
+            value: None,
+        }
+    }
+}
+
+impl<'de, I> de::MapAccess<'de> for MapAccess<'de, I>
+where
+    I: Iterator<Item = &'static str>,
+{
+    type Error = Error;
+
+    fn next_key_seed<K>(&mut self, seed: K) -> Result<Option<K::Value>, Error>
+    where
+        K: de::DeserializeSeed<'de>,
+    {
+        loop {
+            return match self.iter.next() {
+                Some(key) => match self.object.remove(key) {
+                    Some(value) => {
+                        self.value = Some(value);
+                        seed.deserialize(key.into_deserializer()).map(Some)
+                    }
+                    None => continue,
+                },
+                None => Ok(None),
+            };
+        }
+    }
+
+    fn next_value_seed<V>(&mut self, seed: V) -> Result<V::Value, Error>
+    where
+        V: de::DeserializeSeed<'de>,
+    {
+        match self.value.take() {
+            Some(value) => seed.deserialize(value).map_err(Error::from),
+            None => Err(de::Error::custom("value is missing")),
+        }
+    }
+}
+
+#[test]
+fn test_extraction() {
+    use serde::Deserialize;
+
+    use crate::api::schema::StringSchema;
+
+    #[derive(Deserialize)]
+    struct Foo {
+        foo1: String,
+        foo2: String,
+    }
+
+    const SIMPLE_STRING: Schema = StringSchema::new("simple").schema();
+    const FOO_SCHEMA: Schema = ObjectSchema::new(
+        "A Foo",
+        &[
+            ("foo1", false, &SIMPLE_STRING),
+            ("foo2", false, &SIMPLE_STRING),
+        ],
+    )
+    .schema();
+
+    #[derive(Deserialize)]
+    struct Bar {
+        bar1: String,
+        bar2: String,
+    }
+
+    const BAR_SCHEMA: Schema = ObjectSchema::new(
+        "A Bar",
+        &[
+            ("bar1", false, &SIMPLE_STRING),
+            ("bar2", false, &SIMPLE_STRING),
+        ],
+    )
+    .schema();
+
+    let mut data = serde_json::json!({
+        "foo1": "hey1",
+        "foo2": "hey2",
+        "bar1": "there1",
+        "bar2": "there2",
+    });
+
+    let data = data.as_object_mut().unwrap();
+
+    let foo: Foo =
+        Foo::deserialize(ExtractValueDeserializer::try_new(data, &FOO_SCHEMA).unwrap()).unwrap();
+
+    assert!(data.remove("foo1").is_none());
+    assert!(data.remove("foo2").is_none());
+    assert_eq!(foo.foo1, "hey1");
+    assert_eq!(foo.foo2, "hey2");
+
+    let bar =
+        Bar::deserialize(ExtractValueDeserializer::try_new(data, &BAR_SCHEMA).unwrap()).unwrap();
+
+    assert!(data.is_empty());
+    assert_eq!(bar.bar1, "there1");
+    assert_eq!(bar.bar2, "there2");
+}
diff --git a/proxmox/src/api/mod.rs b/proxmox/src/api/mod.rs
index b0b8333..8c6f597 100644
--- a/proxmox/src/api/mod.rs
+++ b/proxmox/src/api/mod.rs
@@ -48,3 +48,5 @@ pub use router::{
 
 #[cfg(feature = "cli")]
 pub mod cli;
+
+pub mod de;
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 11/18] api-macro: object schema entry tuple -> struct
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (9 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 10/18] schema: ExtractValueDeserializer Wolfgang Bumiller
@ 2020-12-18 11:25 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 12/18] api-macro: more tuple refactoring Wolfgang Bumiller
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:25 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/method.rs  | 79 +++++++++++++---------------
 proxmox-api-macro/src/api/mod.rs     | 51 +++++++++++-------
 proxmox-api-macro/src/api/structs.rs | 36 ++++++-------
 3 files changed, 89 insertions(+), 77 deletions(-)

diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index 3e85d8c..d7b0021 100644
--- a/proxmox-api-macro/src/api/method.rs
+++ b/proxmox-api-macro/src/api/method.rs
@@ -263,16 +263,14 @@ fn handle_function_signature(
         };
 
         // For any named type which exists on the function signature...
-        if let Some((_ident, optional, ref mut schema)) =
-            input_schema.find_obj_property_by_ident_mut(&pat.ident.to_string())
-        {
+        if let Some(entry) = input_schema.find_obj_property_by_ident_mut(&pat.ident.to_string()) {
             // try to infer the type in the schema if it is not specified explicitly:
-            let is_option = util::infer_type(schema, &*pat_type.ty)?;
-            let has_default = schema.find_schema_property("default").is_some();
-            if !is_option && *optional && !has_default {
+            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 {
                 error!(pat_type => "optional types need a default or be an Option<T>");
             }
-            if has_default && !*optional {
+            if has_default && !entry.optional {
                 error!(pat_type => "non-optional parameter cannot have a default");
             }
         } else {
@@ -313,40 +311,39 @@ fn handle_function_signature(
         //        bail out with an error.
         let pat_ident = pat.ident.unraw();
         let mut param_name: FieldName = pat_ident.clone().into();
-        let param_type = if let Some((name, optional, schema)) =
-            input_schema.find_obj_property_by_ident(&pat_ident.to_string())
-        {
-            if let SchemaItem::Inferred(span) = &schema.item {
-                bail!(*span, "failed to infer type");
-            }
-            param_name = name.clone();
-            // Found an explicit parameter: extract it:
-            ParameterType::Other(&pat_type.ty, *optional, schema)
-        } else if is_api_method_type(&pat_type.ty) {
-            if api_method_param.is_some() {
-                error!(pat_type => "multiple ApiMethod parameters found");
-                continue;
-            }
-            api_method_param = Some(param_list.len());
-            ParameterType::ApiMethod
-        } else if is_rpc_env_type(&pat_type.ty) {
-            if rpc_env_param.is_some() {
-                error!(pat_type => "multiple RpcEnvironment parameters found");
-                continue;
-            }
-            rpc_env_param = Some(param_list.len());
-            ParameterType::RpcEnv
-        } else if is_value_type(&pat_type.ty) {
-            if value_param.is_some() {
-                error!(pat_type => "multiple additional Value parameters found");
+        let param_type =
+            if let Some(entry) = input_schema.find_obj_property_by_ident(&pat_ident.to_string()) {
+                if let SchemaItem::Inferred(span) = &entry.schema.item {
+                    bail!(*span, "failed to infer type");
+                }
+                param_name = entry.name.clone();
+                // Found an explicit parameter: extract it:
+                ParameterType::Other(&pat_type.ty, entry.optional, &entry.schema)
+            } else if is_api_method_type(&pat_type.ty) {
+                if api_method_param.is_some() {
+                    error!(pat_type => "multiple ApiMethod parameters found");
+                    continue;
+                }
+                api_method_param = Some(param_list.len());
+                ParameterType::ApiMethod
+            } else if is_rpc_env_type(&pat_type.ty) {
+                if rpc_env_param.is_some() {
+                    error!(pat_type => "multiple RpcEnvironment parameters found");
+                    continue;
+                }
+                rpc_env_param = Some(param_list.len());
+                ParameterType::RpcEnv
+            } else if is_value_type(&pat_type.ty) {
+                if value_param.is_some() {
+                    error!(pat_type => "multiple additional Value parameters found");
+                    continue;
+                }
+                value_param = Some(param_list.len());
+                ParameterType::Value
+            } else {
+                error!(&pat_ident => "unexpected parameter {:?}", pat_ident.to_string());
                 continue;
-            }
-            value_param = Some(param_list.len());
-            ParameterType::Value
-        } else {
-            error!(&pat_ident => "unexpected parameter {:?}", pat_ident.to_string());
-            continue;
-        };
+            };
 
         param_list.push((param_name, param_type));
     }
@@ -594,7 +591,7 @@ impl<'a> DefaultParameters<'a> {
     fn get_default(&self, param_tokens: TokenStream) -> Result<syn::Expr, syn::Error> {
         let param_name: syn::LitStr = syn::parse2(param_tokens)?;
         match self.0.find_obj_property_by_ident(&param_name.value()) {
-            Some((_ident, _optional, schema)) => match schema.find_schema_property("default") {
+            Some(entry) => match entry.schema.find_schema_property("default") {
                 Some(def) => Ok(def.clone()),
                 None => bail!(param_name => "no default found in schema"),
             },
diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs
index 3cf1309..4690654 100644
--- a/proxmox-api-macro/src/api/mod.rs
+++ b/proxmox-api-macro/src/api/mod.rs
@@ -176,15 +176,12 @@ impl Schema {
         }
     }
 
-    fn find_obj_property_by_ident(&self, key: &str) -> Option<&(FieldName, bool, Schema)> {
+    fn find_obj_property_by_ident(&self, key: &str) -> Option<&ObjectEntry> {
         self.as_object()
             .and_then(|obj| obj.find_property_by_ident(key))
     }
 
-    fn find_obj_property_by_ident_mut(
-        &mut self,
-        key: &str,
-    ) -> Option<&mut (FieldName, bool, Schema)> {
+    fn find_obj_property_by_ident_mut(&mut self, key: &str) -> Option<&mut ObjectEntry> {
         self.as_object_mut()
             .and_then(|obj| obj.find_property_by_ident_mut(key))
     }
@@ -384,10 +381,26 @@ impl SchemaItem {
     }
 }
 
+pub struct ObjectEntry {
+    pub name: FieldName,
+    pub optional: bool,
+    pub schema: Schema,
+}
+
+impl ObjectEntry {
+    pub fn new(name: FieldName, optional: bool, schema: Schema) -> Self {
+        Self {
+            name,
+            optional,
+            schema,
+        }
+    }
+}
+
 #[derive(Default)]
 /// Contains a sorted list of properties:
 pub struct SchemaObject {
-    properties_: Vec<(FieldName, bool, Schema)>,
+    properties_: Vec<ObjectEntry>,
 }
 
 impl SchemaObject {
@@ -403,12 +416,12 @@ impl SchemaObject {
     }
 
     #[inline]
-    fn properties_mut(&mut self) -> &mut [(FieldName, bool, Schema)] {
+    fn properties_mut(&mut self) -> &mut [ObjectEntry] {
         &mut self.properties_
     }
 
     fn sort_properties(&mut self) {
-        self.properties_.sort_by(|a, b| (a.0).cmp(&b.0));
+        self.properties_.sort_by(|a, b| (a.name).cmp(&b.name));
     }
 
     fn try_extract_from(obj: &mut JSONObject) -> Result<Self, syn::Error> {
@@ -432,7 +445,7 @@ impl SchemaObject {
                             .transpose()?
                             .unwrap_or(false);
 
-                        properties.push((key, optional, schema.try_into()?));
+                        properties.push(ObjectEntry::new(key, optional, schema.try_into()?));
 
                         Ok(properties)
                     },
@@ -444,30 +457,32 @@ impl SchemaObject {
 
     fn to_schema_inner(&self, ts: &mut TokenStream) -> Result<(), Error> {
         for element in self.properties_.iter() {
-            let key = element.0.as_str();
-            let optional = element.1;
+            let key = element.name.as_str();
+            let optional = element.optional;
             let mut schema = TokenStream::new();
-            element.2.to_schema(&mut schema)?;
+            element.schema.to_schema(&mut schema)?;
             ts.extend(quote! { (#key, #optional, &#schema), });
         }
         Ok(())
     }
 
-    fn find_property_by_ident(&self, key: &str) -> Option<&(FieldName, bool, Schema)> {
-        self.properties_.iter().find(|p| p.0.as_ident_str() == key)
+    fn find_property_by_ident(&self, key: &str) -> Option<&ObjectEntry> {
+        self.properties_
+            .iter()
+            .find(|p| p.name.as_ident_str() == key)
     }
 
-    fn find_property_by_ident_mut(&mut self, key: &str) -> Option<&mut (FieldName, bool, Schema)> {
+    fn find_property_by_ident_mut(&mut self, key: &str) -> Option<&mut ObjectEntry> {
         self.properties_
             .iter_mut()
-            .find(|p| p.0.as_ident_str() == key)
+            .find(|p| p.name.as_ident_str() == key)
     }
 
     fn remove_property_by_ident(&mut self, key: &str) -> bool {
         match self
             .properties_
             .iter()
-            .position(|(name, _, _)| name.as_ident_str() == key)
+            .position(|entry| entry.name.as_ident_str() == key)
         {
             Some(index) => {
                 self.properties_.remove(index);
@@ -477,7 +492,7 @@ impl SchemaObject {
         }
     }
 
-    fn extend_properties(&mut self, new_fields: Vec<(FieldName, bool, Schema)>) {
+    fn extend_properties(&mut self, new_fields: Vec<ObjectEntry>) {
         self.properties_.extend(new_fields);
         self.sort_properties();
     }
diff --git a/proxmox-api-macro/src/api/structs.rs b/proxmox-api-macro/src/api/structs.rs
index db6a290..e55378d 100644
--- a/proxmox-api-macro/src/api/structs.rs
+++ b/proxmox-api-macro/src/api/structs.rs
@@ -19,7 +19,7 @@ use proc_macro2::{Ident, Span, TokenStream};
 use quote::quote_spanned;
 
 use super::Schema;
-use crate::api::{self, SchemaItem};
+use crate::api::{self, ObjectEntry, SchemaItem};
 use crate::serde;
 use crate::util::{self, FieldName, JSONObject, Maybe};
 
@@ -126,19 +126,19 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
     //
     // NOTE: We remove references we're "done with" and in the end fail with a list of extraneous
     // fields if there are any.
-    let mut schema_fields: HashMap<String, &mut (FieldName, bool, Schema)> = HashMap::new();
+    let mut schema_fields: HashMap<String, &mut ObjectEntry> = HashMap::new();
 
     // We also keep a reference to the SchemaObject around since we derive missing fields
     // automatically.
     if let api::SchemaItem::Object(ref mut obj) = &mut schema.item {
         for field in obj.properties_mut() {
-            schema_fields.insert(field.0.as_str().to_string(), field);
+            schema_fields.insert(field.name.as_str().to_string(), field);
         }
     } else {
         error!(schema.span, "structs need an object schema");
     }
 
-    let mut new_fields: Vec<(FieldName, bool, Schema)> = Vec::new();
+    let mut new_fields: Vec<ObjectEntry> = Vec::new();
 
     let container_attrs = serde::ContainerAttrib::try_from(&stru.attrs[..])?;
 
@@ -170,19 +170,19 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
                     if attrs.flatten {
                         to_remove.push(name.clone());
 
-                        let name = &field_def.0;
-                        let optional = &field_def.1;
-                        let schema = &field_def.2;
-                        if schema.description.is_explicit() {
+                        if field_def.schema.description.is_explicit() {
                             error!(
-                                name.span(),
+                                field_def.name.span(),
                                 "flattened field should not have a description, \
                                  it does not appear in serialized data as a field",
                             );
                         }
 
-                        if *optional {
-                            error!(name.span(), "optional flattened fields are not supported");
+                        if field_def.optional {
+                            error!(
+                                field_def.name.span(),
+                                "optional flattened fields are not supported"
+                            );
                         }
                     }
 
@@ -190,12 +190,12 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
 
                     if attrs.flatten {
                         all_of_schemas.extend(quote::quote! {&});
-                        field_def.2.to_schema(&mut all_of_schemas)?;
+                        field_def.schema.to_schema(&mut all_of_schemas)?;
                         all_of_schemas.extend(quote::quote! {,});
                     }
                 }
                 None => {
-                    let mut field_def = (
+                    let mut field_def = ObjectEntry::new(
                         FieldName::new(name.clone(), span),
                         false,
                         Schema::blank(span),
@@ -204,7 +204,7 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
 
                     if attrs.flatten {
                         all_of_schemas.extend(quote::quote! {&});
-                        field_def.2.to_schema(&mut all_of_schemas)?;
+                        field_def.schema.to_schema(&mut all_of_schemas)?;
                         all_of_schemas.extend(quote::quote! {,});
                         to_remove.push(name.clone());
                     } else {
@@ -309,11 +309,11 @@ fn handle_regular_struct(attribs: JSONObject, stru: syn::ItemStruct) -> Result<T
 ///
 /// For each field we derive the description from doc-attributes if available.
 fn handle_regular_field(
-    field_def: &mut (FieldName, bool, Schema),
+    field_def: &mut ObjectEntry,
     field: &syn::Field,
     derived: bool, // whether this field was missing in the schema
 ) -> Result<(), Error> {
-    let schema: &mut Schema = &mut field_def.2;
+    let schema: &mut Schema = &mut field_def.schema;
 
     if schema.description.is_none() {
         let (doc_comment, doc_span) = util::get_doc_comments(&field.attrs)?;
@@ -324,8 +324,8 @@ fn handle_regular_field(
 
     if is_option_type(&field.ty) {
         if derived {
-            field_def.1 = true;
-        } else if !field_def.1 {
+            field_def.optional = true;
+        } else if !field_def.optional {
             error!(&field.ty => "non-optional Option type?");
         }
     }
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 12/18] api-macro: more tuple refactoring
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (10 preceding siblings ...)
  2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 11/18] api-macro: object schema entry tuple -> struct Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 13/18] api-macro: factor parameter extraction into a function Wolfgang Bumiller
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/method.rs | 30 ++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index d7b0021..3d6e9ed 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::{Schema, SchemaItem};
+use super::{ObjectEntry, Schema, SchemaItem};
 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
@@ -218,7 +218,12 @@ enum ParameterType<'a> {
     Value,
     ApiMethod,
     RpcEnv,
-    Other(&'a syn::Type, bool, &'a Schema),
+    Normal(NormalParameter<'a>),
+}
+
+struct NormalParameter<'a> {
+    ty: &'a syn::Type,
+    entry: &'a ObjectEntry,
 }
 
 fn check_input_type(input: &syn::FnArg) -> Result<(&syn::PatType, &syn::PatIdent), syn::Error> {
@@ -318,7 +323,10 @@ fn handle_function_signature(
                 }
                 param_name = entry.name.clone();
                 // Found an explicit parameter: extract it:
-                ParameterType::Other(&pat_type.ty, entry.optional, &entry.schema)
+                ParameterType::Normal(NormalParameter {
+                    ty: &pat_type.ty,
+                    entry: &entry,
+                })
             } else if is_api_method_type(&pat_type.ty) {
                 if api_method_param.is_some() {
                     error!(pat_type => "multiple ApiMethod parameters found");
@@ -449,7 +457,7 @@ fn create_wrapper_function(
             ParameterType::Value => args.extend(quote_spanned! { span => input_params, }),
             ParameterType::ApiMethod => args.extend(quote_spanned! { span => api_method_param, }),
             ParameterType::RpcEnv => args.extend(quote_spanned! { span => rpc_env_param, }),
-            ParameterType::Other(ty, optional, schema) => {
+            ParameterType::Normal(param) => {
                 let name_str = syn::LitStr::new(name.as_str(), span);
                 let arg_name =
                     Ident::new(&format!("input_arg_{}", name.as_ident().to_string()), span);
@@ -462,8 +470,8 @@ fn create_wrapper_function(
                         .map(::serde_json::from_value)
                         .transpose()?
                 });
-                let default_value = schema.find_schema_property("default");
-                if !optional {
+                let default_value = param.entry.schema.find_schema_property("default");
+                if !param.entry.optional {
                     // Non-optional types need to be extracted out of the option though (unless
                     // they have a default):
                     //
@@ -477,7 +485,7 @@ fn create_wrapper_function(
                         ))?
                     });
                 }
-                let no_option_type = util::is_option_type(ty).is_none();
+                let no_option_type = util::is_option_type(param.ty).is_none();
 
                 if let Some(def) = &default_value {
                     let name_uc = name.as_ident().to_string().to_uppercase();
@@ -486,20 +494,20 @@ fn create_wrapper_function(
                         span,
                     );
                     // strip possible Option<> from this type:
-                    let ty = util::is_option_type(ty).unwrap_or(ty);
+                    let ty = util::is_option_type(param.ty).unwrap_or(param.ty);
                     default_consts.extend(quote_spanned! { span =>
                         pub const #name: #ty = #def;
                     });
-                    if optional && no_option_type {
+                    if param.entry.optional && no_option_type {
                         // Optional parameter without an Option<T> type requires a default:
                         body.extend(quote_spanned! { span =>
                             .unwrap_or(#name)
                         });
                     }
-                } else if optional && no_option_type {
+                } else if param.entry.optional && no_option_type {
                     // FIXME: we should not be able to reach this without having produced another
                     // error above already anyway?
-                    error!(ty => "Optional parameter without Option<T> requires a default");
+                    error!(param.ty => "Optional parameter without Option<T> requires a default");
                     // we produced an error so just write something that will compile
                     body.extend(quote_spanned! { span =>
                         .unwrap_or_else(|| unreachable!())
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 13/18] api-macro: factor parameter extraction into a function
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (11 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 12/18] api-macro: more tuple refactoring Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 14/18] api-macro: support flattened parameters Wolfgang Bumiller
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/method.rs | 142 +++++++++++++++++-----------
 1 file changed, 85 insertions(+), 57 deletions(-)

diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index 3d6e9ed..ff5d3e0 100644
--- a/proxmox-api-macro/src/api/method.rs
+++ b/proxmox-api-macro/src/api/method.rs
@@ -458,63 +458,15 @@ fn create_wrapper_function(
             ParameterType::ApiMethod => args.extend(quote_spanned! { span => api_method_param, }),
             ParameterType::RpcEnv => args.extend(quote_spanned! { span => rpc_env_param, }),
             ParameterType::Normal(param) => {
-                let name_str = syn::LitStr::new(name.as_str(), span);
-                let arg_name =
-                    Ident::new(&format!("input_arg_{}", name.as_ident().to_string()), span);
-
-                // Optional parameters are expected to be Option<> types in the real function
-                // signature, so we can just keep the returned Option from `input_map.remove()`.
-                body.extend(quote_spanned! { span =>
-                    let #arg_name = input_map
-                        .remove(#name_str)
-                        .map(::serde_json::from_value)
-                        .transpose()?
-                });
-                let default_value = param.entry.schema.find_schema_property("default");
-                if !param.entry.optional {
-                    // Non-optional types need to be extracted out of the option though (unless
-                    // they have a default):
-                    //
-                    // Whether the parameter is optional should have been verified by the schema
-                    // verifier already, so here we just use anyhow::bail! instead of building a
-                    // proper http error!
-                    body.extend(quote_spanned! { span =>
-                        .ok_or_else(|| ::anyhow::format_err!(
-                            "missing non-optional parameter: {}",
-                            #name_str,
-                        ))?
-                    });
-                }
-                let no_option_type = util::is_option_type(param.ty).is_none();
-
-                if let Some(def) = &default_value {
-                    let name_uc = name.as_ident().to_string().to_uppercase();
-                    let name = Ident::new(
-                        &format!("API_METHOD_{}_PARAM_DEFAULT_{}", func_uc, name_uc),
-                        span,
-                    );
-                    // strip possible Option<> from this type:
-                    let ty = util::is_option_type(param.ty).unwrap_or(param.ty);
-                    default_consts.extend(quote_spanned! { span =>
-                        pub const #name: #ty = #def;
-                    });
-                    if param.entry.optional && no_option_type {
-                        // Optional parameter without an Option<T> type requires a default:
-                        body.extend(quote_spanned! { span =>
-                            .unwrap_or(#name)
-                        });
-                    }
-                } else if param.entry.optional && 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<T> requires a default");
-                    // we produced an error so just write something that will compile
-                    body.extend(quote_spanned! { span =>
-                        .unwrap_or_else(|| unreachable!())
-                    });
-                }
-                body.extend(quote_spanned! { span => ; });
-                args.extend(quote_spanned! { span => #arg_name, });
+                extract_normal_parameter(
+                    param,
+                    &mut body,
+                    &mut args,
+                    &func_uc,
+                    name,
+                    span,
+                    default_consts,
+                )?;
             }
         }
     }
@@ -573,6 +525,82 @@ fn create_wrapper_function(
     Ok(api_func_name)
 }
 
+fn extract_normal_parameter(
+    param: NormalParameter,
+    body: &mut TokenStream,
+    args: &mut TokenStream,
+    func_uc: &str,
+    name: FieldName,
+    name_span: Span,
+    default_consts: &mut TokenStream,
+) -> Result<(), Error> {
+    let span = name_span; // renamed during refactorization
+    let name_str = syn::LitStr::new(name.as_str(), span);
+    let arg_name = Ident::new(&format!("input_arg_{}", name.as_ident().to_string()), span);
+
+    // Optional parameters are expected to be Option<> types in the real function
+    // signature, so we can just keep the returned Option from `input_map.remove()`.
+    body.extend(quote_spanned! { span =>
+        let #arg_name = input_map
+            .remove(#name_str)
+            .map(::serde_json::from_value)
+            .transpose()?
+    });
+
+    let default_value = param.entry.schema.find_schema_property("default");
+    if !param.entry.optional {
+        // Non-optional types need to be extracted out of the option though (unless
+        // they have a default):
+        //
+        // Whether the parameter is optional should have been verified by the schema
+        // verifier already, so here we just use anyhow::bail! instead of building a
+        // proper http error!
+        body.extend(quote_spanned! { span =>
+            .ok_or_else(|| ::anyhow::format_err!(
+                "missing non-optional parameter: {}",
+                #name_str,
+            ))?
+        });
+    }
+
+    let no_option_type = util::is_option_type(param.ty).is_none();
+
+    if let Some(def) = &default_value {
+        let name_uc = name.as_ident().to_string().to_uppercase();
+        let name = Ident::new(
+            &format!("API_METHOD_{}_PARAM_DEFAULT_{}", func_uc, name_uc),
+            span,
+        );
+
+        // strip possible Option<> from this type:
+        let ty = util::is_option_type(param.ty).unwrap_or(param.ty);
+        default_consts.extend(quote_spanned! { span =>
+            pub const #name: #ty = #def;
+        });
+
+        if param.entry.optional && no_option_type {
+            // Optional parameter without an Option<T> type requires a default:
+            body.extend(quote_spanned! { span =>
+                .unwrap_or(#name)
+            });
+        }
+    } else if param.entry.optional && 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<T> requires a default");
+
+        // we produced an error so just write something that will compile
+        body.extend(quote_spanned! { span =>
+            .unwrap_or_else(|| unreachable!())
+        });
+    }
+
+    body.extend(quote_spanned! { span => ; });
+    args.extend(quote_spanned! { span => #arg_name, });
+
+    Ok(())
+}
+
 struct DefaultParameters<'a>(&'a Schema);
 
 impl<'a> VisitMut for DefaultParameters<'a> {
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 14/18] api-macro: support flattened parameters
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (12 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 13/18] api-macro: factor parameter extraction into a function Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 15/18] schema: ParameterSchema at 'api' level Wolfgang Bumiller
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/src/api/method.rs | 286 +++++++++++++++++++++-------
 proxmox-api-macro/src/api/mod.rs    |  64 ++++++-
 proxmox-api-macro/tests/allof.rs    | 137 ++++++++++++-
 3 files changed, 414 insertions(+), 73 deletions(-)

diff --git a/proxmox-api-macro/src/api/method.rs b/proxmox-api-macro/src/api/method.rs
index ff5d3e0..23501bc 100644
--- a/proxmox-api-macro/src/api/method.rs
+++ b/proxmox-api-macro/src/api/method.rs
@@ -85,7 +85,7 @@ impl TryFrom<JSONObject> for ReturnType {
 ///
 /// See the top level macro documentation for a complete example.
 pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<TokenStream, Error> {
-    let mut input_schema: Schema = match attribs.remove("input") {
+    let input_schema: Schema = match attribs.remove("input") {
         Some(input) => input.into_object("input schema definition")?.try_into()?,
         None => Schema {
             span: Span::call_site(),
@@ -95,6 +95,19 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
         },
     };
 
+    let mut input_schema = if input_schema.as_object().is_some() {
+        input_schema
+    } else {
+        error!(
+            input_schema.span,
+            "method input schema must be an object schema"
+        );
+        let mut schema = Schema::empty_object(input_schema.span);
+        schema.description = input_schema.description;
+        schema.properties = input_schema.properties;
+        schema
+    };
+
     let mut return_type: Option<ReturnType> = attribs
         .remove("returns")
         .map(|ret| ret.into_object("return schema definition")?.try_into())
@@ -158,25 +171,15 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
     // input schema is done, let's give the method body a chance to extract default parameters:
     DefaultParameters(&input_schema).visit_item_fn_mut(&mut func);
 
-    let input_schema = {
-        let mut ts = TokenStream::new();
-        input_schema.to_typed_schema(&mut ts)?;
-        ts
-    };
-
     let vis = &func.vis;
     let func_name = &func.sig.ident;
     let api_method_name = Ident::new(
         &format!("API_METHOD_{}", func_name.to_string().to_uppercase()),
         func.sig.ident.span(),
     );
-    let input_schema_name = Ident::new(
-        &format!(
-            "API_PARAMETER_SCHEMA_{}",
-            func_name.to_string().to_uppercase()
-        ),
-        func.sig.ident.span(),
-    );
+
+    let (input_schema_code, input_schema_parameter) =
+        serialize_input_schema(input_schema, &func.sig.ident, func.sig.span())?;
 
     let mut returns_schema_setter = TokenStream::new();
     if let Some(return_type) = return_type {
@@ -192,13 +195,12 @@ pub fn handle_method(mut attribs: JSONObject, mut func: syn::ItemFn) -> Result<T
     };
 
     Ok(quote_spanned! { func.sig.span() =>
-        pub const #input_schema_name: ::proxmox::api::schema::ObjectSchema =
-            #input_schema;
+        #input_schema_code
 
         #vis const #api_method_name: ::proxmox::api::ApiMethod =
-            ::proxmox::api::ApiMethod::new(
+            ::proxmox::api::ApiMethod::new_full(
                 &#api_handler,
-                &#input_schema_name,
+                #input_schema_parameter,
             )
             #returns_schema_setter
             #access_setter
@@ -538,69 +540,221 @@ fn extract_normal_parameter(
     let name_str = syn::LitStr::new(name.as_str(), span);
     let arg_name = Ident::new(&format!("input_arg_{}", name.as_ident().to_string()), span);
 
+    let default_value = param.entry.schema.find_schema_property("default");
+
     // Optional parameters are expected to be Option<> types in the real function
     // signature, so we can just keep the returned Option from `input_map.remove()`.
-    body.extend(quote_spanned! { span =>
-        let #arg_name = input_map
-            .remove(#name_str)
-            .map(::serde_json::from_value)
-            .transpose()?
-    });
+    match param.entry.flatten {
+        None => {
+            // regular parameter, we just remove it and call `from_value`.
 
-    let default_value = param.entry.schema.find_schema_property("default");
-    if !param.entry.optional {
-        // Non-optional types need to be extracted out of the option though (unless
-        // they have a default):
-        //
-        // Whether the parameter is optional should have been verified by the schema
-        // verifier already, so here we just use anyhow::bail! instead of building a
-        // proper http error!
-        body.extend(quote_spanned! { span =>
-            .ok_or_else(|| ::anyhow::format_err!(
-                "missing non-optional parameter: {}",
-                #name_str,
-            ))?
-        });
-    }
+            body.extend(quote_spanned! { span =>
+                let #arg_name = input_map
+                    .remove(#name_str)
+                    .map(::serde_json::from_value)
+                    .transpose()?
+            });
 
-    let no_option_type = util::is_option_type(param.ty).is_none();
+            if !param.entry.optional {
+                // Non-optional types need to be extracted out of the option though (unless
+                // they have a default):
+                //
+                // Whether the parameter is optional should have been verified by the schema
+                // verifier already, so here we just use anyhow::bail! instead of building a
+                // proper http error!
+                body.extend(quote_spanned! { span =>
+                    .ok_or_else(|| ::anyhow::format_err!(
+                        "missing non-optional parameter: {}",
+                        #name_str,
+                    ))?
+                });
+            }
 
-    if let Some(def) = &default_value {
-        let name_uc = name.as_ident().to_string().to_uppercase();
-        let name = Ident::new(
-            &format!("API_METHOD_{}_PARAM_DEFAULT_{}", func_uc, name_uc),
-            span,
-        );
+            let no_option_type = util::is_option_type(param.ty).is_none();
 
-        // strip possible Option<> from this type:
-        let ty = util::is_option_type(param.ty).unwrap_or(param.ty);
-        default_consts.extend(quote_spanned! { span =>
-            pub const #name: #ty = #def;
-        });
+            if let Some(def) = &default_value {
+                let name_uc = name.as_ident().to_string().to_uppercase();
+                let name = Ident::new(
+                    &format!("API_METHOD_{}_PARAM_DEFAULT_{}", func_uc, name_uc),
+                    span,
+                );
+
+                // strip possible Option<> from this type:
+                let ty = util::is_option_type(param.ty).unwrap_or(param.ty);
+                default_consts.extend(quote_spanned! { span =>
+                    pub const #name: #ty = #def;
+                });
+
+                if param.entry.optional && no_option_type {
+                    // Optional parameter without an Option<T> type requires a default:
+                    body.extend(quote_spanned! { span =>
+                        .unwrap_or(#name)
+                    });
+                }
+            } else if param.entry.optional && 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<T> requires a default");
+
+                // we produced an error so just write something that will compile
+                body.extend(quote_spanned! { span =>
+                    .unwrap_or_else(|| unreachable!())
+                });
+            }
 
-        if param.entry.optional && no_option_type {
-            // Optional parameter without an Option<T> type requires a default:
-            body.extend(quote_spanned! { span =>
-                .unwrap_or(#name)
-            });
+            body.extend(quote_spanned! { span => ; });
+        }
+        Some(flatten_span) => {
+            // Flattened parameter, we need ot use our special partial-object deserializer.
+            // Also note that we do not support simply nesting schemas. We need a referenced type.
+            // Otherwise the expanded code here gets ugly and we'd need to make sure we pull out
+            // nested schemas into named variables first... No thanks.
+
+            if default_value.is_some() {
+                error!(
+                    default_value =>
+                    "flattened parameter cannot have a default as it cannot be optional",
+                );
+            }
+
+            if let Some(schema_ref) = param.entry.schema.to_schema_reference() {
+                let ty = param.ty;
+                body.extend(quote_spanned! { span =>
+                    let #arg_name = <#ty as ::serde::Deserialize>::deserialize(
+                        ::proxmox::api::de::ExtractValueDeserializer::try_new(
+                            input_map,
+                            #schema_ref,
+                        )
+                        .ok_or_else(|| ::anyhow::format_err!(
+                            "flattened parameter {:?} has invalid schema",
+                            #name_str,
+                        ))?,
+                    )?;
+                });
+            } else {
+                error!(
+                    flatten_span,
+                    "flattened parameter schema must be a schema reference"
+                );
+                body.extend(quote_spanned! { span => let #arg_name = unreachable!(); });
+            }
         }
-    } else if param.entry.optional && 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<T> requires a default");
-
-        // we produced an error so just write something that will compile
-        body.extend(quote_spanned! { span =>
-            .unwrap_or_else(|| unreachable!())
-        });
     }
 
-    body.extend(quote_spanned! { span => ; });
     args.extend(quote_spanned! { span => #arg_name, });
 
     Ok(())
 }
 
+/// Returns a tuple containing the schema code first and the `ParameterSchema` parameter for the
+/// `ApiMethod` second.
+fn serialize_input_schema(
+    mut input_schema: Schema,
+    func_name: &Ident,
+    func_sig_span: Span,
+) -> Result<(TokenStream, TokenStream), Error> {
+    let input_schema_name = Ident::new(
+        &format!(
+            "API_PARAMETER_SCHEMA_{}",
+            func_name.to_string().to_uppercase()
+        ),
+        func_name.span(),
+    );
+
+    let (flattened, has_params) = match &mut input_schema.item {
+        SchemaItem::Object(obj) => {
+            let flattened = obj.drain_filter(|entry| entry.flatten.is_none());
+            (flattened, !obj.is_empty())
+        }
+        _ => (Vec::new(), true),
+    };
+
+    if flattened.is_empty() {
+        let mut ts = TokenStream::new();
+        input_schema.to_typed_schema(&mut ts)?;
+        return Ok((
+            quote_spanned! { func_sig_span =>
+                pub const #input_schema_name: ::proxmox::api::schema::ObjectSchema = #ts;
+            },
+            quote_spanned! { func_sig_span =>
+                ::proxmox::api::router::ParameterSchema::Object(&#input_schema_name)
+            },
+        ));
+    }
+
+    let mut all_of_schemas = TokenStream::new();
+    for entry in flattened {
+        if entry.optional {
+            error!(
+                entry.schema.span,
+                "optional flattened parameters are not supported"
+            );
+        }
+
+        all_of_schemas.extend(quote::quote! {&});
+        entry.schema.to_schema(&mut all_of_schemas)?;
+        all_of_schemas.extend(quote::quote! {,});
+    }
+
+    let description = match input_schema.description.take().ok() {
+        Some(description) => description,
+        None => {
+            error!(input_schema.span, "missing description on api type struct");
+            syn::LitStr::new("<missing description>", input_schema.span)
+        }
+    };
+    // and replace it with a "dummy"
+    input_schema.description = Maybe::Derived(syn::LitStr::new(
+        &format!("<INNER: {}>", description.value()),
+        description.span(),
+    ));
+
+    let (inner_schema, inner_schema_ref) = if has_params {
+        // regular parameters go into the "inner" schema to merge into the AllOfSchema
+        let inner_schema_name = Ident::new(
+            &format!(
+                "API_REGULAR_PARAMETER_SCHEMA_{}",
+                func_name.to_string().to_uppercase()
+            ),
+            func_name.span(),
+        );
+
+        let obj_schema = {
+            let mut ts = TokenStream::new();
+            input_schema.to_schema(&mut ts)?;
+            ts
+        };
+
+        (
+            quote_spanned!(func_sig_span =>
+                const #inner_schema_name: ::proxmox::api::schema::Schema = #obj_schema;
+            ),
+            quote_spanned!(func_sig_span => &#inner_schema_name,),
+        )
+    } else {
+        // otherwise it stays empty
+        (TokenStream::new(), TokenStream::new())
+    };
+
+    Ok((
+        quote_spanned! { func_sig_span =>
+            #inner_schema
+
+            pub const #input_schema_name: ::proxmox::api::schema::AllOfSchema =
+                ::proxmox::api::schema::AllOfSchema::new(
+                    #description,
+                    &[
+                        #inner_schema_ref
+                        #all_of_schemas
+                    ],
+                );
+        },
+        quote_spanned! { func_sig_span =>
+            ::proxmox::api::router::ParameterSchema::AllOf(&#input_schema_name)
+        },
+    ))
+}
+
 struct DefaultParameters<'a>(&'a Schema);
 
 impl<'a> VisitMut for DefaultParameters<'a> {
diff --git a/proxmox-api-macro/src/api/mod.rs b/proxmox-api-macro/src/api/mod.rs
index 4690654..5994f19 100644
--- a/proxmox-api-macro/src/api/mod.rs
+++ b/proxmox-api-macro/src/api/mod.rs
@@ -142,6 +142,17 @@ impl Schema {
         }
     }
 
+    /// Create the token stream for a reference schema (`ExternType` or `ExternSchema`).
+    fn to_schema_reference(&self) -> Option<TokenStream> {
+        match &self.item {
+            SchemaItem::ExternType(path) => {
+                Some(quote_spanned! { path.span() => &#path::API_SCHEMA })
+            }
+            SchemaItem::ExternSchema(path) => Some(quote_spanned! { path.span() => &#path }),
+            _ => None,
+        }
+    }
+
     fn to_typed_schema(&self, ts: &mut TokenStream) -> Result<(), Error> {
         self.item.to_schema(
             ts,
@@ -323,7 +334,8 @@ impl SchemaItem {
             }
             SchemaItem::ExternType(path) => {
                 if !properties.is_empty() {
-                    error!(&properties[0].0 => "additional properties not allowed on external type");
+                    error!(&properties[0].0 =>
+                        "additional properties not allowed on external type");
                 }
                 if let Maybe::Explicit(description) = description {
                     error!(description => "description not allowed on external type");
@@ -385,6 +397,10 @@ pub struct ObjectEntry {
     pub name: FieldName,
     pub optional: bool,
     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<Span>,
 }
 
 impl ObjectEntry {
@@ -393,8 +409,14 @@ impl ObjectEntry {
             name,
             optional,
             schema,
+            flatten: None,
         }
     }
+
+    pub fn with_flatten(mut self, flatten: Option<Span>) -> Self {
+        self.flatten = flatten;
+        self
+    }
 }
 
 #[derive(Default)]
@@ -420,6 +442,24 @@ impl SchemaObject {
         &mut self.properties_
     }
 
+    fn drain_filter<F>(&mut self, mut func: F) -> Vec<ObjectEntry>
+    where
+        F: FnMut(&ObjectEntry) -> bool,
+    {
+        let mut out = Vec::new();
+
+        let mut i = 0;
+        while i != self.properties_.len() {
+            if !func(&self.properties_[i]) {
+                out.push(self.properties_.remove(i));
+            } else {
+                i += 1;
+            }
+        }
+
+        out
+    }
+
     fn sort_properties(&mut self) {
         self.properties_.sort_by(|a, b| (a.name).cmp(&b.name));
     }
@@ -445,7 +485,19 @@ impl SchemaObject {
                             .transpose()?
                             .unwrap_or(false);
 
-                        properties.push(ObjectEntry::new(key, optional, schema.try_into()?));
+                        let flatten: Option<Span> = schema
+                            .remove_entry("flatten")
+                            .map(|(field, value)| -> Result<(Span, bool), syn::Error> {
+                                let v: syn::LitBool = value.try_into()?;
+                                Ok((field.span(), v.value))
+                            })
+                            .transpose()?
+                            .and_then(|(span, value)| if value { Some(span) } else { None });
+
+                        properties.push(
+                            ObjectEntry::new(key, optional, schema.try_into()?)
+                                .with_flatten(flatten),
+                        );
 
                         Ok(properties)
                     },
@@ -457,6 +509,14 @@ impl SchemaObject {
 
     fn to_schema_inner(&self, ts: &mut TokenStream) -> Result<(), Error> {
         for element in self.properties_.iter() {
+            if let Some(span) = element.flatten {
+                error!(
+                    span,
+                    "`flatten` attribute is only available on method parameters, \
+                     use #[serde(flatten)] in structs"
+                );
+            }
+
             let key = element.name.as_str();
             let optional = element.optional;
             let mut schema = TokenStream::new();
diff --git a/proxmox-api-macro/tests/allof.rs b/proxmox-api-macro/tests/allof.rs
index 56e86d7..1c1b9a9 100644
--- a/proxmox-api-macro/tests/allof.rs
+++ b/proxmox-api-macro/tests/allof.rs
@@ -1,10 +1,12 @@
 //! Testing the `AllOf` schema on structs and methods.
 
+use anyhow::Error;
+use serde::{Deserialize, Serialize};
+use serde_json::{json, Value};
+
 use proxmox::api::schema;
 use proxmox_api_macro::api;
 
-use serde::{Deserialize, Serialize};
-
 pub const NAME_SCHEMA: schema::Schema = schema::StringSchema::new("Name.").schema();
 pub const VALUE_SCHEMA: schema::Schema = schema::IntegerSchema::new("Value.").schema();
 pub const INDEX_SCHEMA: schema::Schema = schema::IntegerSchema::new("Index.").schema();
@@ -18,7 +20,7 @@ pub const TEXT_SCHEMA: schema::Schema = schema::StringSchema::new("Text.").schem
 )]
 /// Name and value.
 #[derive(Deserialize, Serialize)]
-struct NameValue {
+pub struct NameValue {
     name: String,
     value: u64,
 }
@@ -31,7 +33,7 @@ struct NameValue {
 )]
 /// Index and text.
 #[derive(Deserialize, Serialize)]
-struct IndexText {
+pub struct IndexText {
     index: u64,
     text: String,
 }
@@ -44,7 +46,7 @@ struct IndexText {
 )]
 /// Name, value, index and text.
 #[derive(Deserialize, Serialize)]
-struct Nvit {
+pub struct Nvit {
     #[serde(flatten)]
     nv: NameValue,
 
@@ -116,3 +118,128 @@ fn test_extra() {
 
     assert_eq!(TEST_SCHEMA, WithExtra::API_SCHEMA);
 }
+
+#[api(
+    input: {
+        properties: {
+            nv: { flatten: true, type: NameValue },
+            it: { flatten: true, type: IndexText },
+        },
+    },
+)]
+/// Hello method.
+pub fn hello(it: IndexText, nv: NameValue) -> Result<(NameValue, IndexText), Error> {
+    Ok((nv, it))
+}
+
+#[test]
+fn hello_schema_check() {
+    const TEST_METHOD: ::proxmox::api::ApiMethod = ::proxmox::api::ApiMethod::new_full(
+        &::proxmox::api::ApiHandler::Sync(&api_function_hello),
+        ::proxmox::api::router::ParameterSchema::AllOf(&::proxmox::api::schema::AllOfSchema::new(
+            "Hello method.",
+            &[&IndexText::API_SCHEMA, &NameValue::API_SCHEMA],
+        )),
+    );
+    assert_eq!(TEST_METHOD, API_METHOD_HELLO);
+}
+
+#[api(
+    input: {
+        properties: {
+            nv: { flatten: true, type: NameValue },
+            it: { flatten: true, type: IndexText },
+            extra: { description: "An extra field." },
+        },
+    },
+)]
+/// Extra method.
+pub fn with_extra(
+    it: IndexText,
+    nv: NameValue,
+    extra: String,
+) -> Result<(NameValue, IndexText, String), Error> {
+    Ok((nv, it, extra))
+}
+
+#[test]
+fn with_extra_schema_check() {
+    const INNER_SCHEMA: ::proxmox::api::schema::Schema = ::proxmox::api::schema::ObjectSchema::new(
+        "<INNER: Extra method.>",
+        &[(
+            "extra",
+            false,
+            &::proxmox::api::schema::StringSchema::new("An extra field.").schema(),
+        )],
+    )
+    .schema();
+
+    const TEST_METHOD: ::proxmox::api::ApiMethod = ::proxmox::api::ApiMethod::new_full(
+        &::proxmox::api::ApiHandler::Sync(&api_function_with_extra),
+        ::proxmox::api::router::ParameterSchema::AllOf(&::proxmox::api::schema::AllOfSchema::new(
+            "Extra method.",
+            &[
+                &INNER_SCHEMA,
+                &IndexText::API_SCHEMA,
+                &NameValue::API_SCHEMA,
+            ],
+        )),
+    );
+    assert_eq!(TEST_METHOD, API_METHOD_WITH_EXTRA);
+}
+
+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<String>) {
+        let _ = user;
+        panic!("set_auth_id called");
+    }
+
+    /// Get authentication id
+    fn get_auth_id(&self) -> Option<String> {
+        panic!("get_auth_id called");
+    }
+}
+
+#[test]
+fn test_invocations() {
+    let mut env = RpcEnv;
+    let value = api_function_hello(
+        json!({"name":"Bob", "value":3, "index":4, "text":"Text"}),
+        &API_METHOD_HELLO,
+        &mut env,
+    )
+    .expect("hello function should work");
+
+    assert_eq!(value[0]["name"], "Bob");
+    assert_eq!(value[0]["value"], 3);
+    assert_eq!(value[1]["index"], 4);
+    assert_eq!(value[1]["text"], "Text");
+
+    let value = api_function_with_extra(
+        json!({"name":"Alice", "value":8, "index":2, "text":"Paragraph", "extra":"Some Extra"}),
+        &API_METHOD_WITH_EXTRA,
+        &mut env,
+    )
+    .expect("`with_extra` function should work");
+
+    assert_eq!(value[0]["name"], "Alice");
+    assert_eq!(value[0]["value"], 8);
+    assert_eq!(value[1]["index"], 2);
+    assert_eq!(value[1]["text"], "Paragraph");
+    assert_eq!(value[2], "Some Extra");
+}
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 15/18] schema: ParameterSchema at 'api' level
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (13 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 14/18] api-macro: support flattened parameters Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 16/18] proxmox: temporary d/changelog update Wolfgang Bumiller
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/src/api/mod.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/proxmox/src/api/mod.rs b/proxmox/src/api/mod.rs
index 8c6f597..5319cc1 100644
--- a/proxmox/src/api/mod.rs
+++ b/proxmox/src/api/mod.rs
@@ -43,7 +43,8 @@ pub mod router;
 #[cfg(feature = "router")]
 #[doc(inline)]
 pub use router::{
-    ApiFuture, ApiHandler, ApiMethod, ApiResponseFuture, Router, SubRoute, SubdirMap,
+    ApiFuture, ApiHandler, ApiMethod, ApiResponseFuture, ParameterSchema, Router, SubRoute,
+    SubdirMap,
 };
 
 #[cfg(feature = "cli")]
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 16/18] proxmox: temporary d/changelog update
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (14 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 15/18] schema: ParameterSchema at 'api' level Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 17/18] macro: " Wolfgang Bumiller
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/debian/changelog | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/proxmox/debian/changelog b/proxmox/debian/changelog
index 5e8c449..050619d 100644
--- a/proxmox/debian/changelog
+++ b/proxmox/debian/changelog
@@ -1,3 +1,13 @@
+rust-proxmox (0.9.0-1) UNRELEASED; urgency=medium
+
+  * `ApiMethod.returns` is now a `router::ReturnType` rather than a direct
+    `&Schema` reference and can be marked as `optional`.
+
+  * Added an `AllOfSchema` (`Schema::AllOf`) where multiple object schemas can
+    be combined like with JSONSchema/openapi's `allOf` property.
+
+ -- Proxmox Support Team <support@proxmox.com>  Fri, 11 Dec 2020 14:55:29 +0100
+
 rust-proxmox (0.8.1-1) unstable; urgency=medium
 
   * trait ReadExt: add read_exact_or_eof and skip_to_end
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 17/18] macro: temporary d/changelog update
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (15 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 16/18] proxmox: temporary d/changelog update Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 18/18] proxmox changelog update Wolfgang Bumiller
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox-api-macro/debian/changelog | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/proxmox-api-macro/debian/changelog b/proxmox-api-macro/debian/changelog
index 0c72dce..c514474 100644
--- a/proxmox-api-macro/debian/changelog
+++ b/proxmox-api-macro/debian/changelog
@@ -1,3 +1,13 @@
+rust-proxmox-api-macro (0.3.0-1) UNRELEASED; urgency=medium
+
+  * removed `pub const API_RETURN_SCHEMA_*` generation
+    This could already be accessed via the public `API_METHOD_FOO.returns`.
+
+  * Note that a previous `schema: API_RETURN_SCHEMA_FOO` must now dereference
+    the schema via: `schema: *API_METHOD_FOO.returns.schema`.
+
+ -- Proxmox Support Team <support@proxmox.com>  Fri, 11 Dec 2020 14:56:02 +0100
+
 rust-proxmox-api-macro (0.2.4-1) unstable; urgency=medium
 
   * support raw parameter name identifiers (eg. `r#type`)
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH proxmox 18/18] proxmox changelog update
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (16 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 17/18] macro: " Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH backup 1/2] adaptions for proxmox 0.9 and proxmox-api-macro 0.3 Wolfgang Bumiller
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 proxmox/debian/changelog | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/proxmox/debian/changelog b/proxmox/debian/changelog
index 050619d..3505bfc 100644
--- a/proxmox/debian/changelog
+++ b/proxmox/debian/changelog
@@ -6,6 +6,14 @@ rust-proxmox (0.9.0-1) UNRELEASED; urgency=medium
   * Added an `AllOfSchema` (`Schema::AllOf`) where multiple object schemas can
     be combined like with JSONSchema/openapi's `allOf` property.
 
+  * `ApiMethod.parameters` is now a `ParameterSchema` instead of simply an
+    `ObjectSchema`.
+
+  * There's now an `ObjectSchemaType` trait implemented by `ObjectSchema`,
+    `AllOfSchema` as well as `ParameterSchema` for simplicity. Some of the
+    verifiers/parsers now use the trait to cope with both types with minimal
+    changes.
+
  -- Proxmox Support Team <support@proxmox.com>  Fri, 11 Dec 2020 14:55:29 +0100
 
 rust-proxmox (0.8.1-1) unstable; urgency=medium
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH backup 1/2] adaptions for proxmox 0.9 and proxmox-api-macro 0.3
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (17 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 18/18] proxmox changelog update Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-18 11:26 ` [pbs-devel] [PATCH backup 2/2] tests: verify-api: check AllOf schemas Wolfgang Bumiller
  2020-12-22  6:45 ` [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Dietmar Maurer
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 src/api2/admin/datastore.rs                    |  8 ++++----
 src/bin/proxmox-backup-client.rs               | 12 ++++++------
 src/bin/proxmox-backup-manager.rs              | 12 ++++++------
 src/bin/proxmox-tape.rs                        |  4 ++--
 src/bin/proxmox_backup_client/benchmark.rs     |  5 +++--
 src/bin/proxmox_backup_client/key.rs           | 10 ++++++++--
 src/bin/proxmox_backup_client/snapshot.rs      |  9 +++++----
 src/bin/proxmox_backup_client/task.rs          |  4 ++--
 src/bin/proxmox_backup_manager/acl.rs          |  2 +-
 src/bin/proxmox_backup_manager/datastore.rs    |  4 ++--
 src/bin/proxmox_backup_manager/disk.rs         |  8 ++++----
 src/bin/proxmox_backup_manager/dns.rs          |  2 +-
 src/bin/proxmox_backup_manager/network.rs      |  2 +-
 src/bin/proxmox_backup_manager/remote.rs       |  4 ++--
 src/bin/proxmox_backup_manager/subscription.rs |  2 +-
 src/bin/proxmox_backup_manager/sync.rs         |  4 ++--
 src/bin/proxmox_backup_manager/user.rs         |  4 ++--
 src/bin/proxmox_tape/changer.rs                |  8 ++++----
 src/bin/proxmox_tape/drive.rs                  |  6 +++---
 src/bin/proxmox_tape/media.rs                  |  2 +-
 src/bin/proxmox_tape/pool.rs                   |  4 ++--
 src/server/rest.rs                             |  9 +++++----
 22 files changed, 67 insertions(+), 58 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 16fee943..32352e5c 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -15,7 +15,7 @@ use proxmox::api::{
     api, ApiResponseFuture, ApiHandler, ApiMethod, Router,
     RpcEnvironment, RpcEnvironmentType, Permission
 };
-use proxmox::api::router::SubdirMap;
+use proxmox::api::router::{ReturnType, SubdirMap};
 use proxmox::api::schema::*;
 use proxmox::tools::fs::{replace_file, CreateOptions};
 use proxmox::{http_err, identity, list_subdirs_api_method, sortable};
@@ -148,7 +148,7 @@ fn get_all_snapshot_files(
     },
 )]
 /// List backup groups.
-fn list_groups(
+pub fn list_groups(
     store: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Vec<GroupListItem>, Error> {
@@ -772,7 +772,7 @@ pub const API_RETURN_SCHEMA_PRUNE: Schema = ArraySchema::new(
     &PruneListItem::API_SCHEMA
 ).schema();
 
-const API_METHOD_PRUNE: ApiMethod = ApiMethod::new(
+pub const API_METHOD_PRUNE: ApiMethod = ApiMethod::new(
     &ApiHandler::Sync(&prune),
     &ObjectSchema::new(
         "Prune the datastore.",
@@ -787,7 +787,7 @@ const API_METHOD_PRUNE: ApiMethod = ApiMethod::new(
             ("store", false, &DATASTORE_SCHEMA),
         ])
     ))
-    .returns(&API_RETURN_SCHEMA_PRUNE)
+    .returns(ReturnType::new(false, &API_RETURN_SCHEMA_PRUNE))
     .access(None, &Permission::Privilege(
     &["datastore", "{store}"],
     PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_PRUNE,
diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs
index 6cf81952..b8f09a4a 100644
--- a/src/bin/proxmox-backup-client.rs
+++ b/src/bin/proxmox-backup-client.rs
@@ -412,9 +412,9 @@ async fn list_backup_groups(param: Value) -> Result<Value, Error> {
 
     let mut data: Value = result["data"].take();
 
-    let info = &proxmox_backup::api2::admin::datastore::API_RETURN_SCHEMA_LIST_GROUPS;
+    let return_type = &proxmox_backup::api2::admin::datastore::API_METHOD_LIST_GROUPS.returns;
 
-    format_and_print_result_full(&mut data, info, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -1458,7 +1458,7 @@ async fn prune_async(mut param: Value) -> Result<Value, Error> {
         .column(ColumnConfig::new("keep").renderer(render_prune_action).header("action"))
         ;
 
-    let info = &proxmox_backup::api2::admin::datastore::API_RETURN_SCHEMA_PRUNE;
+    let return_type = &proxmox_backup::api2::admin::datastore::API_METHOD_PRUNE.returns;
 
     let mut data = result["data"].take();
 
@@ -1469,7 +1469,7 @@ async fn prune_async(mut param: Value) -> Result<Value, Error> {
         data = list.into();
     }
 
-    format_and_print_result_full(&mut data, info, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -1522,9 +1522,9 @@ async fn status(param: Value) -> Result<Value, Error> {
         .column(ColumnConfig::new("used").renderer(render_total_percentage))
         .column(ColumnConfig::new("avail").renderer(render_total_percentage));
 
-    let schema = &API_RETURN_SCHEMA_STATUS;
+    let return_type = &API_METHOD_STATUS.returns;
 
-    format_and_print_result_full(&mut data, schema, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 8ad4c7dc..ff2a1dc1 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -106,11 +106,11 @@ async fn garbage_collection_status(param: Value) -> Result<Value, Error> {
 
     let mut result = client.get(&path, None).await?;
     let mut data = result["data"].take();
-    let schema = &api2::admin::datastore::API_RETURN_SCHEMA_GARBAGE_COLLECTION_STATUS;
+    let return_type = &api2::admin::datastore::API_METHOD_GARBAGE_COLLECTION_STATUS.returns;
 
     let options = default_table_format_options();
 
-    format_and_print_result_full(&mut data, schema, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -172,7 +172,7 @@ async fn task_list(param: Value) -> Result<Value, Error> {
     let mut result = client.get("api2/json/nodes/localhost/tasks", Some(args)).await?;
 
     let mut data = result["data"].take();
-    let schema = &api2::node::tasks::API_RETURN_SCHEMA_LIST_TASKS;
+    let return_type = &api2::node::tasks::API_METHOD_LIST_TASKS.returns;
 
     let options = default_table_format_options()
         .column(ColumnConfig::new("starttime").right_align(false).renderer(tools::format::render_epoch))
@@ -180,7 +180,7 @@ async fn task_list(param: Value) -> Result<Value, Error> {
         .column(ColumnConfig::new("upid"))
         .column(ColumnConfig::new("status").renderer(tools::format::render_task_status));
 
-    format_and_print_result_full(&mut data, schema, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -370,9 +370,9 @@ async fn get_versions(verbose: bool, param: Value) -> Result<Value, Error> {
         .column(ColumnConfig::new("Version"))
         .column(ColumnConfig::new("ExtraInfo").header("Extra Info"))
         ;
-    let schema = &crate::api2::node::apt::API_RETURN_SCHEMA_GET_VERSIONS;
+    let return_type = &crate::api2::node::apt::API_METHOD_GET_VERSIONS.returns;
 
-    format_and_print_result_full(&mut packages, schema, &output_format, &options);
+    format_and_print_result_full(&mut packages, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox-tape.rs b/src/bin/proxmox-tape.rs
index 9ed9aca9..5c3e8036 100644
--- a/src/bin/proxmox-tape.rs
+++ b/src/bin/proxmox-tape.rs
@@ -311,7 +311,7 @@ async fn read_label(
         .column(ColumnConfig::new("media-set-ctime").renderer(render_epoch))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
@@ -383,7 +383,7 @@ async fn inventory(
         .column(ColumnConfig::new("uuid"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
diff --git a/src/bin/proxmox_backup_client/benchmark.rs b/src/bin/proxmox_backup_client/benchmark.rs
index e53e43ce..2ecb3dc9 100644
--- a/src/bin/proxmox_backup_client/benchmark.rs
+++ b/src/bin/proxmox_backup_client/benchmark.rs
@@ -15,6 +15,7 @@ use proxmox::api::{
         format_and_print_result_full,
         default_table_format_options,
     },
+    router::ReturnType,
 };
 
 use proxmox_backup::backup::{
@@ -178,7 +179,7 @@ fn render_result(
 ) -> Result<(), Error> {
 
     let mut data = serde_json::to_value(benchmark_result)?;
-    let schema = &BenchmarkResult::API_SCHEMA;
+    let return_type = ReturnType::new(false, &BenchmarkResult::API_SCHEMA);
 
     let render_speed = |value: &Value, _record: &Value| -> Result<String, Error> {
         match value["speed"].as_f64() {
@@ -211,7 +212,7 @@ fn render_result(
                 .right_align(false).renderer(render_speed));
 
 
-    format_and_print_result_full(&mut data, schema, output_format, &options);
+    format_and_print_result_full(&mut data, &return_type, output_format, &options);
 
     Ok(())
 }
diff --git a/src/bin/proxmox_backup_client/key.rs b/src/bin/proxmox_backup_client/key.rs
index e49131c1..109f0384 100644
--- a/src/bin/proxmox_backup_client/key.rs
+++ b/src/bin/proxmox_backup_client/key.rs
@@ -15,6 +15,7 @@ use proxmox::api::cli::{
     get_output_format,
     OUTPUT_FORMAT,
 };
+use proxmox::api::router::ReturnType;
 use proxmox::sys::linux::tty;
 use proxmox::tools::fs::{file_get_contents, replace_file, CreateOptions};
 
@@ -382,9 +383,14 @@ fn show_key(
         .column(ColumnConfig::new("modified").renderer(tools::format::render_epoch))
         .column(ColumnConfig::new("fingerprint"));
 
-    let schema = &KeyInfo::API_SCHEMA;
+    let return_type = ReturnType::new(false, &KeyInfo::API_SCHEMA);
 
-    format_and_print_result_full(&mut serde_json::to_value(info)?, schema, &output_format, &options);
+    format_and_print_result_full(
+        &mut serde_json::to_value(info)?,
+        &return_type,
+        &output_format,
+        &options,
+    );
 
     Ok(())
 }
diff --git a/src/bin/proxmox_backup_client/snapshot.rs b/src/bin/proxmox_backup_client/snapshot.rs
index 7be0480f..3bdc5f33 100644
--- a/src/bin/proxmox_backup_client/snapshot.rs
+++ b/src/bin/proxmox_backup_client/snapshot.rs
@@ -97,9 +97,9 @@ async fn list_snapshots(param: Value) -> Result<Value, Error> {
         .column(ColumnConfig::new("files").renderer(render_files))
         ;
 
-    let info = &proxmox_backup::api2::admin::datastore::API_RETURN_SCHEMA_LIST_SNAPSHOTS;
+    let return_type = &proxmox_backup::api2::admin::datastore::API_METHOD_LIST_SNAPSHOTS.returns;
 
-    format_and_print_result_full(&mut data, info, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -144,13 +144,14 @@ async fn list_snapshot_files(param: Value) -> Result<Value, Error> {
 
     record_repository(&repo);
 
-    let info = &proxmox_backup::api2::admin::datastore::API_RETURN_SCHEMA_LIST_SNAPSHOT_FILES;
+    let return_type =
+        &proxmox_backup::api2::admin::datastore::API_METHOD_LIST_SNAPSHOT_FILES.returns;
 
     let mut data: Value = result["data"].take();
 
     let options = default_table_format_options();
 
-    format_and_print_result_full(&mut data, info, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_client/task.rs b/src/bin/proxmox_backup_client/task.rs
index 6f567f22..e4adaf58 100644
--- a/src/bin/proxmox_backup_client/task.rs
+++ b/src/bin/proxmox_backup_client/task.rs
@@ -64,7 +64,7 @@ async fn task_list(param: Value) -> Result<Value, Error> {
     let mut result = client.get("api2/json/nodes/localhost/tasks", Some(args)).await?;
     let mut data = result["data"].take();
 
-    let schema = &proxmox_backup::api2::node::tasks::API_RETURN_SCHEMA_LIST_TASKS;
+    let return_type = &proxmox_backup::api2::node::tasks::API_METHOD_LIST_TASKS.returns;
 
     let options = default_table_format_options()
         .column(ColumnConfig::new("starttime").right_align(false).renderer(tools::format::render_epoch))
@@ -72,7 +72,7 @@ async fn task_list(param: Value) -> Result<Value, Error> {
         .column(ColumnConfig::new("upid"))
         .column(ColumnConfig::new("status").renderer(tools::format::render_task_status));
 
-    format_and_print_result_full(&mut data, schema, &output_format, &options);
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/acl.rs b/src/bin/proxmox_backup_manager/acl.rs
index 7511c8cb..b23943ca 100644
--- a/src/bin/proxmox_backup_manager/acl.rs
+++ b/src/bin/proxmox_backup_manager/acl.rs
@@ -47,7 +47,7 @@ fn list_acls(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Err
         .column(ColumnConfig::new("propagate"))
         .column(ColumnConfig::new("roleid"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index 94590aaf..7e596a1b 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -32,7 +32,7 @@ fn list_datastores(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
         .column(ColumnConfig::new("path"))
         .column(ColumnConfig::new("comment"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -62,7 +62,7 @@ fn show_datastore(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value
     };
 
     let options = default_table_format_options();
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/disk.rs b/src/bin/proxmox_backup_manager/disk.rs
index 70de4775..164f8831 100644
--- a/src/bin/proxmox_backup_manager/disk.rs
+++ b/src/bin/proxmox_backup_manager/disk.rs
@@ -59,7 +59,7 @@ fn list_disks(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value
         .column(ColumnConfig::new("status"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -100,7 +100,7 @@ fn smart_attributes(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result
     let mut data = data["attributes"].take();
 
     let options = default_table_format_options();
-    format_and_print_result_full(&mut data, API_METHOD_SMART_ATTRIBUTES.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &API_METHOD_SMART_ATTRIBUTES.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -227,7 +227,7 @@ fn list_zpools(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
         .column(ColumnConfig::new("alloc").right_align(true).renderer(render_usage))
         .column(ColumnConfig::new("health"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -274,7 +274,7 @@ fn list_datastore_mounts(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> R
         .column(ColumnConfig::new("filesystem"))
         .column(ColumnConfig::new("options"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/dns.rs b/src/bin/proxmox_backup_manager/dns.rs
index c735b22e..9a91cb06 100644
--- a/src/bin/proxmox_backup_manager/dns.rs
+++ b/src/bin/proxmox_backup_manager/dns.rs
@@ -35,7 +35,7 @@ fn get_dns(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, E
         .column(ColumnConfig::new("dns2"))
         .column(ColumnConfig::new("dns3"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/network.rs b/src/bin/proxmox_backup_manager/network.rs
index d7f6382f..c9489f7d 100644
--- a/src/bin/proxmox_backup_manager/network.rs
+++ b/src/bin/proxmox_backup_manager/network.rs
@@ -88,7 +88,7 @@ fn list_network_devices(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Re
         .column(ColumnConfig::new("gateway").header("gateway").renderer(render_gateway))
         .column(ColumnConfig::new("bridge_ports").header("ports/slaves").renderer(render_ports));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/remote.rs b/src/bin/proxmox_backup_manager/remote.rs
index 04d714ef..b1c4aa45 100644
--- a/src/bin/proxmox_backup_manager/remote.rs
+++ b/src/bin/proxmox_backup_manager/remote.rs
@@ -34,7 +34,7 @@ fn list_remotes(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value,
         .column(ColumnConfig::new("fingerprint"))
         .column(ColumnConfig::new("comment"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -64,7 +64,7 @@ fn show_remote(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, E
     };
 
     let options = default_table_format_options();
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/subscription.rs b/src/bin/proxmox_backup_manager/subscription.rs
index 79813a35..cd69d8d3 100644
--- a/src/bin/proxmox_backup_manager/subscription.rs
+++ b/src/bin/proxmox_backup_manager/subscription.rs
@@ -27,7 +27,7 @@ fn get(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
     };
 
     let options = default_table_format_options();
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/sync.rs b/src/bin/proxmox_backup_manager/sync.rs
index f21ecb5f..f05f0c8d 100644
--- a/src/bin/proxmox_backup_manager/sync.rs
+++ b/src/bin/proxmox_backup_manager/sync.rs
@@ -35,7 +35,7 @@ fn list_sync_jobs(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value
         .column(ColumnConfig::new("schedule"))
         .column(ColumnConfig::new("comment"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -65,7 +65,7 @@ fn show_sync_job(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value,
     };
 
     let options = default_table_format_options();
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_backup_manager/user.rs b/src/bin/proxmox_backup_manager/user.rs
index 516d27f2..8d78b08c 100644
--- a/src/bin/proxmox_backup_manager/user.rs
+++ b/src/bin/proxmox_backup_manager/user.rs
@@ -46,7 +46,7 @@ fn list_users(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Er
         .column(ColumnConfig::new("email"))
         .column(ColumnConfig::new("comment"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
@@ -87,7 +87,7 @@ fn list_tokens(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, E
         )
         .column(ColumnConfig::new("comment"));
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(Value::Null)
 }
diff --git a/src/bin/proxmox_tape/changer.rs b/src/bin/proxmox_tape/changer.rs
index 0a8b794e..9db1644e 100644
--- a/src/bin/proxmox_tape/changer.rs
+++ b/src/bin/proxmox_tape/changer.rs
@@ -104,7 +104,7 @@ fn list_changers(
         .column(ColumnConfig::new("serial"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
@@ -139,7 +139,7 @@ fn scan_for_changers(
         .column(ColumnConfig::new("serial"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
@@ -175,7 +175,7 @@ fn get_config(
         .column(ColumnConfig::new("path"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
@@ -213,7 +213,7 @@ async fn get_status(
         .column(ColumnConfig::new("loaded-slot"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
diff --git a/src/bin/proxmox_tape/drive.rs b/src/bin/proxmox_tape/drive.rs
index 0949d450..73e34afc 100644
--- a/src/bin/proxmox_tape/drive.rs
+++ b/src/bin/proxmox_tape/drive.rs
@@ -108,7 +108,7 @@ fn list_drives(
         .column(ColumnConfig::new("serial"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
@@ -143,7 +143,7 @@ fn scan_for_drives(
         .column(ColumnConfig::new("serial"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
@@ -182,7 +182,7 @@ fn get_config(
         .column(ColumnConfig::new("changer-drive-id"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
diff --git a/src/bin/proxmox_tape/media.rs b/src/bin/proxmox_tape/media.rs
index 374cb891..0767bdb0 100644
--- a/src/bin/proxmox_tape/media.rs
+++ b/src/bin/proxmox_tape/media.rs
@@ -106,7 +106,7 @@ async fn list_media(
         .column(ColumnConfig::new("media-set-uuid"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
diff --git a/src/bin/proxmox_tape/pool.rs b/src/bin/proxmox_tape/pool.rs
index 23e8e83e..4d477aaf 100644
--- a/src/bin/proxmox_tape/pool.rs
+++ b/src/bin/proxmox_tape/pool.rs
@@ -92,7 +92,7 @@ fn list_pools(
         .column(ColumnConfig::new("template"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
@@ -131,7 +131,7 @@ fn get_config(
         .column(ColumnConfig::new("template"))
         ;
 
-    format_and_print_result_full(&mut data, info.returns, &output_format, &options);
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
 
     Ok(())
 }
diff --git a/src/server/rest.rs b/src/server/rest.rs
index da110507..c1c4fd55 100644
--- a/src/server/rest.rs
+++ b/src/server/rest.rs
@@ -26,13 +26,14 @@ use proxmox::api::{
     ApiHandler,
     ApiMethod,
     HttpError,
+    ParameterSchema,
     Permission,
     RpcEnvironment,
     RpcEnvironmentType,
     check_api_permission,
 };
 use proxmox::api::schema::{
-    ObjectSchema,
+    ObjectSchemaType,
     parse_parameter_strings,
     parse_simple_value,
     verify_json_object,
@@ -233,7 +234,7 @@ impl tower_service::Service<Request<Body>> for ApiService {
 }
 
 fn parse_query_parameters<S: 'static + BuildHasher + Send>(
-    param_schema: &ObjectSchema,
+    param_schema: ParameterSchema,
     form: &str, // x-www-form-urlencoded body data
     parts: &Parts,
     uri_param: &HashMap<String, String, S>,
@@ -264,7 +265,7 @@ fn parse_query_parameters<S: 'static + BuildHasher + Send>(
 }
 
 async fn get_request_parameters<S: 'static + BuildHasher + Send>(
-    param_schema: &ObjectSchema,
+    param_schema: ParameterSchema,
     parts: Parts,
     req_body: Body,
     uri_param: HashMap<String, String, S>,
@@ -305,7 +306,7 @@ async fn get_request_parameters<S: 'static + BuildHasher + Send>(
                 params[&k] = parse_simple_value(&v, prop_schema)?;
             }
         }
-        verify_json_object(&params, param_schema)?;
+        verify_json_object(&params, &param_schema)?;
         return Ok(params);
     } else {
         parse_query_parameters(param_schema, utf8_data, &parts, &uri_param)
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* [pbs-devel] [PATCH backup 2/2] tests: verify-api: check AllOf schemas
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (18 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH backup 1/2] adaptions for proxmox 0.9 and proxmox-api-macro 0.3 Wolfgang Bumiller
@ 2020-12-18 11:26 ` Wolfgang Bumiller
  2020-12-22  6:45 ` [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Dietmar Maurer
  20 siblings, 0 replies; 23+ messages in thread
From: Wolfgang Bumiller @ 2020-12-18 11:26 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
---
 tests/verify-api.rs | 46 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/tests/verify-api.rs b/tests/verify-api.rs
index 944d24cb..7b7371f6 100644
--- a/tests/verify-api.rs
+++ b/tests/verify-api.rs
@@ -1,3 +1,5 @@
+use std::collections::HashSet;
+
 use anyhow::{bail, format_err, Error};
 
 use proxmox_backup::api2;
@@ -31,11 +33,41 @@ fn verify_object_schema(schema: &ObjectSchema) -> Result<(), Error> {
     Ok(())
 }
 
+// verify entries in an AllOf schema are actually object schemas and that they don't contain
+// duplicate keys
+fn verify_all_of_schema(schema: &AllOfSchema) -> Result<(), Error> {
+    for entry in schema.list {
+        match entry {
+            Schema::Object(obj) => verify_object_schema(obj)?,
+            _ => bail!("AllOf schema with a non-object schema entry!"),
+        }
+    }
+
+    let mut keys = HashSet::<&'static str>::new();
+    let mut dupes = String::new();
+    for property in schema.properties() {
+        if keys.insert(property.0) {
+            if dupes.is_empty() {
+                dupes.push_str(", ");
+            }
+            dupes.push_str(property.0);
+        }
+    }
+    if !dupes.is_empty() {
+        bail!("Duplicate keys found in AllOf schema: {}", dupes);
+    }
+
+    Ok(())
+}
+
 fn verify_schema(schema: &Schema) -> Result<(), Error> {
     match schema {
         Schema::Object(obj_schema) => {
             verify_object_schema(obj_schema)?;
         }
+        Schema::AllOf(all_of_schema) => {
+            verify_all_of_schema(all_of_schema)?;
+        }
         Schema::Array(arr_schema) => {
             verify_schema(arr_schema.items)?;
         }
@@ -68,10 +100,18 @@ fn verify_api_method(
     info: &ApiMethod
 ) -> Result<(), Error>
 {
-    verify_object_schema(info.parameters)
-        .map_err(|err| format_err!("{} {} parameters: {}", method, path, err))?;
+    match &info.parameters {
+        ParameterSchema::Object(obj) => {
+            verify_object_schema(obj)
+                .map_err(|err| format_err!("{} {} parameters: {}", method, path, err))?;
+        }
+        ParameterSchema::AllOf(all_of) => {
+            verify_all_of_schema(all_of)
+                .map_err(|err| format_err!("{} {} parameters: {}", method, path, err))?;
+        }
+    }
 
-    verify_schema(info.returns)
+    verify_schema(info.returns.schema)
         .map_err(|err| format_err!("{} {} returns: {}", method, path, err))?;
 
     verify_access_permissions(info.access.permission)
-- 
2.20.1





^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema
  2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
                   ` (19 preceding siblings ...)
  2020-12-18 11:26 ` [pbs-devel] [PATCH backup 2/2] tests: verify-api: check AllOf schemas Wolfgang Bumiller
@ 2020-12-22  6:45 ` Dietmar Maurer
  2020-12-22  6:51   ` Dietmar Maurer
  20 siblings, 1 reply; 23+ messages in thread
From: Dietmar Maurer @ 2020-12-22  6:45 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Wolfgang Bumiller

applied, but now I am  unable to build the debian packages:

# make deb
./build.sh proxmox
Something failed: ErrorMessage { msg: "failed to find proxmox v0.9.0 (/home/dietmar/rust/proxmox/proxmox) in path source" }
make: *** [Makefile:21: proxmox-deb] Error 1


any ideas?




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema
  2020-12-22  6:45 ` [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Dietmar Maurer
@ 2020-12-22  6:51   ` Dietmar Maurer
  0 siblings, 0 replies; 23+ messages in thread
From: Dietmar Maurer @ 2020-12-22  6:51 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Wolfgang Bumiller

Ignore me, I found the problem

Versions in Cargo.toml needed an update...

> On 12/22/2020 7:45 AM Dietmar Maurer <dietmar@proxmox.com> wrote:
> 
>  
> applied, but now I am  unable to build the debian packages:
> 
> # make deb
> ./build.sh proxmox
> Something failed: ErrorMessage { msg: "failed to find proxmox v0.9.0 (/home/dietmar/rust/proxmox/proxmox) in path source" }
> make: *** [Makefile:21: proxmox-deb] Error 1
> 
> 
> any ideas?
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2020-12-22  6:52 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 11:25 [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 01/18] formatting fixup Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 02/18] schema: support optional return values Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 03/18] api-macro: " Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 04/18] schema: support AllOf schemas Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 05/18] schema: allow AllOf schema as method parameter Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 06/18] api-macro: add 'flatten' to SerdeAttrib Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 07/18] api-macro: forbid flattened fields Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 08/18] api-macro: add more standard Maybe methods Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 09/18] api-macro: suport AllOf on structs Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 10/18] schema: ExtractValueDeserializer Wolfgang Bumiller
2020-12-18 11:25 ` [pbs-devel] [PATCH proxmox 11/18] api-macro: object schema entry tuple -> struct Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 12/18] api-macro: more tuple refactoring Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 13/18] api-macro: factor parameter extraction into a function Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 14/18] api-macro: support flattened parameters Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 15/18] schema: ParameterSchema at 'api' level Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 16/18] proxmox: temporary d/changelog update Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 17/18] macro: " Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH proxmox 18/18] proxmox changelog update Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH backup 1/2] adaptions for proxmox 0.9 and proxmox-api-macro 0.3 Wolfgang Bumiller
2020-12-18 11:26 ` [pbs-devel] [PATCH backup 2/2] tests: verify-api: check AllOf schemas Wolfgang Bumiller
2020-12-22  6:45 ` [pbs-devel] [PATCH proxmox 00/18] Optional Return Types and AllOf schema Dietmar Maurer
2020-12-22  6:51   ` Dietmar Maurer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal