[pbs-devel] [PATCH proxmox 03/18] api-macro: support optional return values

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

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