[pbs-devel] [PATCH proxmox 04/18] schema: support AllOf schemas

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

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