add the ParseCli trait

In order to get parameters from the command line into the
API we need to get them into a json value, so that we can
pass it down the handler which deserializes them into their
real type and runs verifications.
For this we need to define how the type is going to be
converted into a json Value. We cannot simply use
Deserialize as that would for instance require quotes
around strings. So instead, we have a ParseCli trait, which
is a nop (direct serde_json::Value::String()) for string
types, and uses .parse() (the std::str::FromStr trait) for
everything else.

Currently this uses a `default fn` as an example of the
specialization feature, but I'll probably remove this and
use yet another mass-impl macro since there isn't much
activity on that feature's issue tracker. (The issue itself
seems to be outdated ...

Signed-off-by: Wolfgang Bumiller <wry.git@bumiller.com>
This commit is contained in:
Wolfgang Bumiller 2019-06-15 11:41:14 +02:00 committed by Wolfgang Bumiller
parent dc3b88f50c
commit 691af5cade
6 changed files with 92 additions and 23 deletions

View File

@ -448,6 +448,7 @@ fn handle_struct_unnamed(
name: stringify!(#name),
description: "FIXME",
complete_fn: None, // FIXME!
parse_cli: Some(<#name as ::proxmox::api::cli::ParseCli>::parse_cli),
};
&INFO
}
@ -493,6 +494,7 @@ fn handle_struct_named(
name: stringify!(#name),
description: #description,
complete_fn: None, // FIXME!
parse_cli: Some(<#name as ::proxmox::api::cli::ParseCli>::parse_cli),
};
&INFO
}

View File

@ -45,8 +45,12 @@ impl Parameter {
/// Parse a commnd line option: if it is None, we only saw an `--option` without value, this is
/// fine for booleans. If we saw a value, we should try to parse it out into a json value. For
/// string parameters this means passing them as is, for others it means using FromStr...
pub fn parse_cli(&self, _value: Option<&str>) -> Result<Value, Error> {
bail!("TODO");
pub fn parse_cli(&self, name: &str, value: Option<&str>) -> Result<Value, Error> {
let info = (self.type_info)();
match info.parse_cli {
Some(func) => func(name, value),
None => bail!("cannot parse parameter '{}' as command line parameter", name),
}
}
}
@ -57,6 +61,7 @@ pub struct TypeInfo {
pub name: &'static str,
pub description: &'static str,
pub complete_fn: Option<CompleteFn>,
pub parse_cli: Option<fn(name: &str, value: Option<&str>) -> Result<Value, Error>>,
}
impl TypeInfo {
@ -135,24 +140,6 @@ impl<Body> dyn ApiMethodInfo<Body> + Send + Sync {
/// While this is very useful for structural types, we sometimes to want to be able to pass a
/// simple unconstrainted type like a `String` with no restrictions, so most basic types implement
/// `ApiType` as well.
//
// FIXME: I've actually moved most of this into the types in `api_type.rs` now, so this is
// probably unused at this point?
// `verify` should be moved to `TypeInfo` (for the type related verifier), and `Parameter` should
// get an additional verify method for constraints added by *methods*.
//
// We actually have 2 layers of validation:
// When entering the API: The type validation
// obviously a `String` should also be a string in the json object...
// This does not happen when we call the method from rust-code as we have no json layer
// there.
// When entering the function: The input validation
// if the function says `Integer`, the type itself has no validation other than that it has
// to be an integer type, but the function may still say `minimum: 5, maximum: 10`.
// This should also happen for direct calls from within rust, the `#[api]` macro can take
// care of this.
// When leaving the function: The output validation
// Yep, we need to add this ;-)
pub trait ApiType {
/// API types need to provide a `TypeInfo`, providing details about the underlying type.
fn type_info() -> &'static TypeInfo;
@ -210,7 +197,8 @@ impl<T: ApiType> ApiType for Option<T> {
DATA.info.set(Some(TypeInfo {
name: unsafe { (*DATA.name.as_ptr()).as_ref().unwrap().as_str() },
description: unsafe { (*DATA.description.as_ptr()).as_ref().unwrap().as_str() },
complete_fn: None,
complete_fn: info.complete_fn,
parse_cli: info.parse_cli,
}));
});
unsafe { (*DATA.info.as_ptr()).as_ref().unwrap() }
@ -249,6 +237,7 @@ macro_rules! unconstrained_api_type {
name: stringify!($type),
description: stringify!($type),
complete_fn: None,
parse_cli: Some(<$type as $crate::cli::ParseCli>::parse_cli),
};
&INFO
}
@ -276,6 +265,7 @@ impl<Body> ApiType for Response<Body> {
name: "http::Response<>",
description: "A raw http response",
complete_fn: None,
parse_cli: None,
};
&INFO
}

View File

@ -1,8 +1,10 @@
//! Provides Command Line Interface to API methods
use std::collections::HashMap;
use std::str::FromStr;
use failure::{bail, format_err, Error};
use serde::Serialize;
use serde_json::Value;
use super::{ApiMethodInfo, ApiOutput, Parameter};
@ -223,7 +225,7 @@ impl<Body: 'static> Method<Body> {
let param_def = self
.find_parameter(arg)
.ok_or_else(|| format_err!("no such parameter: '{}'", arg))?;
params.insert(arg.to_string(), param_def.parse_cli(value)?);
params.insert(arg.to_string(), param_def.parse_cli(arg, value)?);
Ok(())
}
@ -252,3 +254,70 @@ fn next_arg<'a>(args: &mut std::slice::Iter<&'a str>) -> Option<Arg<'a>> {
}
})
}
pub fn parse_cli_from_str<T>(name: &str, value: Option<&str>) -> Result<Value, Error>
where
T: FromStr + Serialize,
<T as FromStr>::Err: Into<Error>,
{
let this: T = value
.ok_or_else(|| format_err!("missing parameter value for '{}'", name))?
.parse()
.map_err(|e: <T as FromStr>::Err| e.into())?;
Ok(serde_json::to_value(this)?)
}
/// We use this trait so we can keep the "mass implementation macro" for the ApiType trait simple
/// and specialize the CLI parameter parsing via this trait separately.
pub trait ParseCli {
fn parse_cli(name: &str, value: Option<&str>) -> Result<Value, Error>;
}
// Saves us another mass impl macro such as the one below:
impl<T> ParseCli for T {
default fn parse_cli(name: &str, _value: Option<&str>) -> Result<Value, Error> {
bail!("invalid type for command line interface found for parameter '{}'", name);
}
}
#[macro_export]
macro_rules! impl_parse_cli_from_str {
($type:ty $(, $more:ty)*) => {
impl $crate::cli::ParseCli for $type {
fn parse_cli(name: &str, value: Option<&str>) -> Result<Value, Error> {
parse_cli_from_str::<$type>(name, value)
}
}
$crate::impl_parse_cli_from_str!{$($more),*}
};
() => {};
}
impl_parse_cli_from_str! {bool}
impl_parse_cli_from_str! {isize, usize, i64, u64, i32, u32, i16, u16, i8, u8, f64, f32}
impl ParseCli for Value {
fn parse_cli(name: &str, _value: Option<&str>) -> Result<Value, Error> {
// FIXME: we could of course allow generic json parameters...?
bail!("found generic json parameter ('{}') in command line...", name);
}
}
impl ParseCli for &str {
fn parse_cli(name: &str, value: Option<&str>) -> Result<Value, Error> {
Ok(Value::String(value
.ok_or_else(|| format_err!("missing value for parameter '{}'", name))?
.to_string()
))
}
}
impl ParseCli for String {
fn parse_cli(name: &str, value: Option<&str>) -> Result<Value, Error> {
Ok(Value::String(value
.ok_or_else(|| format_err!("missing value for parameter '{}'", name))?
.to_string()
))
}
}

View File

@ -6,6 +6,14 @@
//! Note that you'll rarely need the [`Router`] type itself, as you'll most likely be creating them
//! with the `router` macro provided by the `proxmox-api-macro` crate.
// This saves us some repetition (which we could do via macros), but this makes the code shorter
// and easier to review.
// FIXME: While the RFC has been approved a while ago and the implementation is there, there isn't
// much activity on the issue tracker for this, so should we remove this?
// Currently this is only used in cli.rs for a `default fn` which could instead made explicit for
// our types
#![feature(specialization)]
use std::future::Future;
use std::pin::Pin;

View File

@ -2,7 +2,6 @@
use std::collections::HashMap;
use failure::Error;
use serde_json::{json, Value};
use super::ApiMethodInfo;

View File

@ -165,6 +165,7 @@ mod methods {
name: "Thing",
description: "A thing",
complete_fn: None,
parse_cli: None,
};
&INFO
}