[pbs-devel] [PATCH proxmox 05/18] schema: allow AllOf schema as method parameter

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

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