macro: implement minimum and maximum verification

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

proxmox-api-macro/src/api_macro.rs

@@ -86,13 +86,18 @@ fn handle_function(
         .transpose()?
         .unwrap_or_else(HashMap::new);
     let mut parameter_entries = TokenStream::new();
+    let mut parameter_verifiers = TokenStream::new();
 
     let vis = std::mem::replace(&mut item.vis, syn::Visibility::Inherited);
     let span = item.ident.span();
     let name_str = item.ident.to_string();
-    let impl_str = format!("{}_impl", name_str);
+    //let impl_str = format!("{}_impl", name_str);
-    let impl_ident = Ident::new(&impl_str, span);
+    //let impl_ident = Ident::new(&impl_str, span);
-    let name = std::mem::replace(&mut item.ident, impl_ident.clone());
+    let impl_checked_str = format!("{}_checked_impl", name_str);
+    let impl_checked_ident = Ident::new(&impl_checked_str, span);
+    let impl_unchecked_str = format!("{}_unchecked_impl", name_str);
+    let impl_unchecked_ident = Ident::new(&impl_unchecked_str, span);
+    let name = std::mem::replace(&mut item.ident, impl_unchecked_ident.clone());
     let mut return_type = match item.decl.output {
         syn::ReturnType::Default => syn::Type::Tuple(syn::TypeTuple {
             paren_token: syn::token::Paren {
@@ -148,8 +153,26 @@ fn handle_function(
                 });
             }
             Expression::Expr(_) => bail!("description must be a string literal!"),
-            Expression::Object(_) => {
+            Expression::Object(mut param_info) => {
-                bail!("TODO: parameters with more than just a description...");
+                let description = param_info
+                    .remove("description")
+                    .ok_or_else(|| format_err!("missing 'description' in parameter definition"))?
+                    .expect_lit_str()?;
+
+                parameter_entries.extend(quote! {
+                    ::proxmox::api::Parameter {
+                        name: #name_str,
+                        description: #description,
+                        type_info: <#arg_type as ::proxmox::api::ApiType>::type_info,
+                    },
+                });
+
+                make_parameter_verifier(
+                    &name,
+                    &name_str,
+                    &mut param_info,
+                    &mut parameter_verifiers,
+                )?;
             }
         }
     }
@@ -201,7 +224,7 @@ fn handle_function(
 
         // Namespace some of our code into the helper type:
         impl #struct_name {
-            // This is the original function, renamed to `#impl_ident`
+            // This is the original function, renamed to `#impl_unchecked_ident`
             #item
 
             // This is the handler used by our router, which extracts the parameters out of a
@@ -238,7 +261,7 @@ fn handle_function(
                         ::failure::bail!("unexpected extra parameters: {}", extra);
                     }
 
-                    let output = #struct_name::#impl_ident(#extracted_args).await?;
+                    let output = #struct_name::#impl_checked_ident(#extracted_args).await?;
                     ::proxmox::api::IntoApiOutput::into_api_output(output)
                 }
                 Box::pin(handler(args))
@@ -249,6 +272,13 @@ fn handle_function(
     if item.asyncness.is_some() {
         // An async function is expected to return its value, so we wrap it a bit:
         body.push(quote! {
+            impl #struct_name {
+                async fn #impl_checked_ident(#inputs) -> #return_type {
+                    #parameter_verifiers
+                    Self::#impl_unchecked_ident(#passed_args).await
+                }
+            }
+
             // Our helper type derefs to a wrapper performing input validation and returning a
             // Pin<Box<Future>>.
             // Unfortunately we cannot return the actual function since that won't work for
@@ -262,7 +292,7 @@ fn handle_function(
                     const FUNC: fn(#inputs) -> ::std::pin::Pin<Box<dyn ::std::future::Future<
                         Output = #return_type,
                     >>> = |#inputs| {
-                        Box::pin(#struct_name::#impl_ident(#passed_args))
+                        Box::pin(#struct_name::#impl_checked_ident(#passed_args))
                     };
                     &FUNC
                 }
@@ -284,6 +314,13 @@ fn handle_function(
         });
 
         body.push(quote! {
+            impl #struct_name {
+                fn #impl_checked_ident(#inputs) -> ::proxmox::api::ApiFuture<#body_type> {
+                    #parameter_verifiers
+                    Self::#impl_unchecked_ident(#passed_args)
+                }
+            }
+
             // Our helper type derefs to a wrapper performing input validation and returning a
             // Pin<Box<Future>>.
             // Unfortunately we cannot return the actual function since that won't work for
@@ -292,10 +329,7 @@ fn handle_function(
                 type Target = fn(#inputs) -> ::proxmox::api::ApiFuture<#body_type>;
 
                 fn deref(&self) -> &Self::Target {
-                    const FUNC: fn(#inputs) -> ::proxmox::api::ApiFuture<#body_type> = |#inputs| {
+                    &(Self::#impl_checked_ident as Self::Target)
-                        #struct_name::#impl_ident(#passed_args)
-                    };
-                    &FUNC
                 }
             }
         });
@@ -340,6 +374,37 @@ fn handle_function(
     Ok(body)
 }
 
+fn make_parameter_verifier(
+    var: &Ident,
+    var_str: &str,
+    info: &mut HashMap<String, Expression>,
+    out: &mut TokenStream,
+) -> Result<(), Error> {
+    match info.remove("minimum") {
+        None => (),
+        Some(Expression::Expr(expr)) => out.extend(quote! {
+            let cmp = #expr;
+            if #var < cmp {
+                bail!("parameter '{}' is out of range (must be >= {})", #var_str, cmp);
+            }
+        }),
+        Some(_) => bail!("invalid value for 'minimum'"),
+    }
+
+    match info.remove("maximum") {
+        None => (),
+        Some(Expression::Expr(expr)) => out.extend(quote! {
+            let cmp = #expr;
+            if #var > cmp {
+                bail!("parameter '{}' is out of range (must be <= {})", #var_str, cmp);
+            }
+        }),
+        Some(_) => bail!("invalid value for 'maximum'"),
+    }
+
+    Ok(())
+}
+
 fn handle_struct(
     definition: HashMap<String, Expression>,
     item: &syn::ItemStruct,

proxmox-api-macro/tests/verification.rs

@@ -0,0 +1,137 @@
+#![feature(async_await)]
+
+use bytes::Bytes;
+use failure::{bail, Error};
+use serde_json::{json, Value};
+
+use proxmox::api::{api, Router};
+
+#[api({
+    body: Bytes,
+    description: "A test function returning a fixed text",
+    parameters: {
+        number: {
+            description: "A number",
+            minimum: 3,
+            maximum: 10,
+        },
+        reference: {
+            description: "A reference number",
+            minimum: 3,
+            maximum: 10,
+        },
+    },
+})]
+async fn less_than(number: usize, reference: usize) -> Result<bool, Error> {
+    Ok(number < reference)
+}
+
+proxmox_api_macro::router! {
+    static TEST_ROUTER: Router<Bytes> = {
+        GET: less_than,
+    };
+}
+
+fn check_parameter(
+    router: &Router<Bytes>,
+    path: &str,
+    parameters: Value,
+    expect: Result<&'static str, &'static str>,
+) {
+    let (router, _) = router
+        .lookup(path)
+        .expect("expected method to exist on test router");
+    let method = router
+        .get
+        .as_ref()
+        .expect("expected GET method on router at path");
+    let fut = method.handler()(parameters);
+    match (futures::executor::block_on(fut), expect) {
+        (Ok(resp), Ok(exp)) => {
+            assert_eq!(resp.status(), 200, "test response should have status 200");
+            let body = resp.into_body();
+            let body = std::str::from_utf8(&body).expect("expected test body to be valid utf8");
+            assert_eq!(body, exp, "expected successful output");
+        }
+        (Err(resp), Err(exp)) => {
+            assert_eq!(resp.to_string(), exp.to_string(), "expected specific error");
+        }
+        (Ok(resp), Err(exp)) => {
+            let body = resp.into_body();
+            let body = std::str::from_utf8(&body).expect("expected test body to be valid utf8");
+            panic!(
+                "expected function to fail with `{}`, but it succeeded with `{}`",
+                exp, body
+            );
+        }
+        (Err(resp), Ok(exp)) => {
+            panic!(
+                "expected function to succeed with `{}`, but it failed with `{}`",
+                exp, resp
+            );
+        }
+    }
+}
+
+#[test]
+fn router() {
+    // Expected successes:
+    check_parameter(
+        &TEST_ROUTER,
+        "/",
+        json!({
+            "number": 3,
+            "reference": 5,
+        }),
+        Ok(r#"{"data":true}"#),
+    );
+
+    check_parameter(
+        &TEST_ROUTER,
+        "/",
+        json!({
+            "number": 5,
+            "reference": 5,
+        }),
+        Ok(r#"{"data":false}"#),
+    );
+
+    // Expected failures:
+    check_parameter(
+        &TEST_ROUTER,
+        "/",
+        json!({
+            "number": 1,
+            "reference": 5,
+        }),
+        Err("parameter 'number' is out of range (must be >= 3)"),
+    );
+
+    check_parameter(
+        &TEST_ROUTER,
+        "/",
+        json!({
+            "number": 3,
+            "reference": 2,
+        }),
+        Err("parameter 'reference' is out of range (must be >= 3)"),
+    );
+
+    check_parameter(
+        &TEST_ROUTER,
+        "/",
+        json!({
+            "number": 3,
+            "reference": 20,
+        }),
+        Err("parameter 'reference' is out of range (must be <= 10)"),
+    );
+
+    //// And can I...
+    let res = futures::executor::block_on(less_than(1, 5)).map_err(|x| x.to_string());
+    assert_eq!(
+        res,
+        Err("parameter 'number' is out of range (must be >= 3)".to_string()),
+        "expected FOO from direct get_loopback('FOO') call"
+    );
+}