konevsa/proxmox
1
0
Fork 0
forked from Proxmox/proxmox
Code Pull Requests Activity

api: make method body an associated type

Browse Source 
This way we do not need to carry the body type into the CLI
router and can instead just require the body to be
Into<Bytes>.

This also makes more sense, because previously a method
could in theory implement multiple ApiMethodInfo types with
different bodies which seems pointless.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
This commit is contained in:
Wolfgang Bumiller 2019-06-19 15:16:40 +02:00
parent dbcadda6b2
commit bd79dd8f02
6 changed files with 85 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
proxmox-api-macro/src/api_macro.rs
View File

@ -341,7 +341,9 @@ fn handle_function(
// //
// Note that technically we don't need the `description` member in this trait, as this is // Note that technically we don't need the `description` member in this trait, as this is
// mostly used at compile time for documentation! // mostly used at compile time for documentation!
impl ::proxmox::api::ApiMethodInfo<#body_type> for #struct_name { impl ::proxmox::api::ApiMethodInfo for #struct_name {
type Body = #body_type;
fn description(&self) -> &'static str { fn description(&self) -> &'static str {
#fn_api_description #fn_api_description
} }

2
proxmox-api/Cargo.toml
View File

@ -5,6 +5,7 @@ version = "0.1.0"
authors = [ "Wolfgang Bumiller <w.bumiller@proxmox.com>" ] authors = [ "Wolfgang Bumiller <w.bumiller@proxmox.com>" ]
[dependencies] [dependencies]
bytes = "0.4"
failure = "0.1" failure = "0.1"
futures-preview = "0.3.0-alpha" futures-preview = "0.3.0-alpha"
http = "0.1" http = "0.1"
@ -13,5 +14,4 @@ serde_derive = "1.0"
serde_json = "1.0" serde_json = "1.0"
[dev-dependencies] [dev-dependencies]
bytes = "0.4"
lazy_static = "1.3" lazy_static = "1.3"

12
proxmox-api/src/api_type.rs
View File

@ -7,13 +7,15 @@ use serde_json::{json, Value};
/// Method entries in a `Router` are actually just `&dyn ApiMethodInfo` trait objects. /// Method entries in a `Router` are actually just `&dyn ApiMethodInfo` trait objects.
/// This contains all the info required to call, document, or command-line-complete parameters for /// This contains all the info required to call, document, or command-line-complete parameters for
/// a method. /// a method.
pub trait ApiMethodInfo<Body> { pub trait ApiMethodInfo {
type Body;
fn description(&self) -> &'static str; fn description(&self) -> &'static str;
fn parameters(&self) -> &'static [Parameter]; fn parameters(&self) -> &'static [Parameter];
fn return_type(&self) -> &'static TypeInfo; fn return_type(&self) -> &'static TypeInfo;
fn protected(&self) -> bool; fn protected(&self) -> bool;
fn reload_timezone(&self) -> bool; fn reload_timezone(&self) -> bool;
fn handler(&self) -> fn(Value) -> super::ApiFuture<Body>; fn handler(&self) -> fn(Value) -> super::ApiFuture<Self::Body>;
} }
/// Shortcut to not having to type it out. This function signature is just a dummy and not yet /// Shortcut to not having to type it out. This function signature is just a dummy and not yet
@ -84,7 +86,9 @@ pub struct ApiMethod<Body> {
pub handler: fn(Value) -> super::ApiFuture<Body>, pub handler: fn(Value) -> super::ApiFuture<Body>,
} }
impl<Body> ApiMethodInfo<Body> for ApiMethod<Body> { impl<Body> ApiMethodInfo for ApiMethod<Body> {
type Body = Body;
fn description(&self) -> &'static str { fn description(&self) -> &'static str {
self.description self.description
} }
@ -110,7 +114,7 @@ impl<Body> ApiMethodInfo<Body> for ApiMethod<Body> {
} }
} }
impl<Body> dyn ApiMethodInfo<Body> + Send + Sync { impl<Body> dyn ApiMethodInfo<Body = Body> + Send + Sync {
pub fn api_dump(&self) -> Value { pub fn api_dump(&self) -> Value {
let parameters = Value::Object(std::iter::FromIterator::from_iter( let parameters = Value::Object(std::iter::FromIterator::from_iter(
self.parameters() self.parameters()

102
proxmox-api/src/cli.rs
View File

@ -3,21 +3,22 @@
use std::collections::HashMap; use std::collections::HashMap;
use std::str::FromStr; use std::str::FromStr;
use bytes::Bytes;
use failure::{bail, format_err, Error}; use failure::{bail, format_err, Error};
use serde::Serialize; use serde::Serialize;
use serde_json::Value; use serde_json::Value;
use super::{ApiMethodInfo, ApiOutput, Parameter}; use super::{ApiMethodInfo, ApiOutput, Parameter};
type MethodInfoRef<Body> = &'static (dyn ApiMethodInfo<Body> + Send + Sync); type MethodInfoRef = &'static dyn UnifiedApiMethod;
/// A CLI root node. /// A CLI root node.
pub struct App<Body: 'static> { pub struct App {
name: &'static str, name: &'static str,
command: Option<Command<Body>>, command: Option<Command>,
} }
impl<Body: 'static> App<Body> { impl App {
/// Create a new empty App instance. /// Create a new empty App instance.
pub fn new(name: &'static str) -> Self { pub fn new(name: &'static str) -> Self {
Self { Self {
@ -29,7 +30,7 @@ impl<Body: 'static> App<Body> {
/// Directly connect this instance to a single API method. /// Directly connect this instance to a single API method.
/// ///
/// This is a builder method and will panic if there's already a method registered! /// This is a builder method and will panic if there's already a method registered!
pub fn method(mut self, method: Method<Body>) -> Self { pub fn method(mut self, method: Method) -> Self {
assert!( assert!(
self.command.is_none(), self.command.is_none(),
"app {} already has a comman!", "app {} already has a comman!",
@ -44,7 +45,7 @@ impl<Body: 'static> App<Body> {
/// ///
/// This is a builder method and will panic if the subcommand already exists or no subcommands /// This is a builder method and will panic if the subcommand already exists or no subcommands
/// may be added. /// may be added.
pub fn subcommand(mut self, name: &'static str, subcommand: Command<Body>) -> Self { pub fn subcommand(mut self, name: &'static str, subcommand: Command) -> Self {
match self match self
.command .command
.get_or_insert_with(|| Command::SubCommands(SubCommands::new())) .get_or_insert_with(|| Command::SubCommands(SubCommands::new()))
@ -58,7 +59,7 @@ impl<Body: 'static> App<Body> {
} }
/// Resolve a list of parameters to a method and a parameter json value. /// Resolve a list of parameters to a method and a parameter json value.
pub fn resolve(&self, args: &[&str]) -> Result<(MethodInfoRef<Body>, Value), Error> { pub fn resolve(&self, args: &[&str]) -> Result<(MethodInfoRef, Value), Error> {
self.command self.command
.as_ref() .as_ref()
.ok_or_else(|| format_err!("no commands available"))? .ok_or_else(|| format_err!("no commands available"))?
@ -66,25 +67,29 @@ impl<Body: 'static> App<Body> {
} }
/// Run a command through this command line interface. /// Run a command through this command line interface.
pub fn run(&self, args: &[&str]) -> ApiOutput<Body> { pub fn run(&self, args: &[&str]) -> ApiOutput<Bytes> {
let (method, params) = self.resolve(args)?; let (method, params) = self.resolve(args)?;
let handler = method.handler(); let future = method.call(params);
futures::executor::block_on(handler(params)) futures::executor::block_on(future)
} }
} }
/// A node in the CLI command router. This is either /// A node in the CLI command router. This is either
pub enum Command<Body: 'static> { pub enum Command {
Method(Method<Body>), Method(Method),
SubCommands(SubCommands<Body>), SubCommands(SubCommands),
} }
impl<Body: 'static> Command<Body> { impl Command {
/// Create a Command entry pointing to an API method /// Create a Command entry pointing to an API method
pub fn method( pub fn method<T: Send + Sync>(
method: &'static (dyn ApiMethodInfo<Body> + Send + Sync), method: &'static T,
positional_args: &'static [&'static str], positional_args: &'static [&'static str],
) -> Self { ) -> Self
where
T: ApiMethodInfo,
T::Body: 'static + Into<Bytes>,
{
Command::Method(Method::new(method, positional_args)) Command::Method(Method::new(method, positional_args))
} }
@ -93,7 +98,7 @@ impl<Body: 'static> Command<Body> {
Command::SubCommands(SubCommands::new()) Command::SubCommands(SubCommands::new())
} }
fn resolve(&self, args: std::slice::Iter<&str>) -> Result<(MethodInfoRef<Body>, Value), Error> { fn resolve(&self, args: std::slice::Iter<&str>) -> Result<(MethodInfoRef, Value), Error> {
match self { match self {
Command::Method(method) => method.resolve(args), Command::Method(method) => method.resolve(args),
Command::SubCommands(subcmd) => subcmd.resolve(args), Command::SubCommands(subcmd) => subcmd.resolve(args),
@ -101,11 +106,11 @@ impl<Body: 'static> Command<Body> {
} }
} }
pub struct SubCommands<Body: 'static> { pub struct SubCommands {
commands: HashMap<&'static str, Command<Body>>, commands: HashMap<&'static str, Command>,
} }
impl<Body: 'static> SubCommands<Body> { impl SubCommands {
/// Create a new empty SubCommands hash. /// Create a new empty SubCommands hash.
pub fn new() -> Self { pub fn new() -> Self {
Self { Self {
@ -116,7 +121,7 @@ impl<Body: 'static> SubCommands<Body> {
/// Add a subcommand. /// Add a subcommand.
/// ///
/// Note that it is illegal for the subcommand to already exist, which will cause a panic. /// Note that it is illegal for the subcommand to already exist, which will cause a panic.
pub fn add_subcommand(&mut self, name: &'static str, command: Command<Body>) -> &mut Self { pub fn add_subcommand(&mut self, name: &'static str, command: Command) -> &mut Self {
let old = self.commands.insert(name, command); let old = self.commands.insert(name, command);
assert!(old.is_none(), "subcommand '{}' already exists", name); assert!(old.is_none(), "subcommand '{}' already exists", name);
self self
@ -125,15 +130,12 @@ impl<Body: 'static> SubCommands<Body> {
/// Builder method to add a subcommand. /// Builder method to add a subcommand.
/// ///
/// Note that it is illegal for the subcommand to already exist, which will cause a panic. /// Note that it is illegal for the subcommand to already exist, which will cause a panic.
pub fn subcommand(mut self, name: &'static str, command: Command<Body>) -> Self { pub fn subcommand(mut self, name: &'static str, command: Command) -> Self {
self.add_subcommand(name, command); self.add_subcommand(name, command);
self self
} }
fn resolve( fn resolve(&self, mut args: std::slice::Iter<&str>) -> Result<(MethodInfoRef, Value), Error> {
&self,
mut args: std::slice::Iter<&str>,
) -> Result<(MethodInfoRef<Body>, Value), Error> {
match args.next() { match args.next() {
None => bail!("missing subcommand"), None => bail!("missing subcommand"),
Some(arg) => match self.commands.get(arg) { Some(arg) => match self.commands.get(arg) {
@ -144,6 +146,36 @@ impl<Body: 'static> SubCommands<Body> {
} }
} }
/// API methods can have different body types. For the CLI we don't care whether it is a
/// hyper::Body or a bytes::Bytes (also because we don't care for partia bodies etc.), so the
/// output needs to be wrapped to a common format. So basically the CLI will only ever see
/// `ApiOutput<Bytes>`.
pub trait UnifiedApiMethod: Send + Sync {
fn parameters(&self) -> &'static [Parameter];
fn call(&self, params: Value) -> super::ApiFuture<Bytes>;
}
impl<T: Send + Sync> UnifiedApiMethod for T
where
T: ApiMethodInfo,
T::Body: 'static + Into<Bytes>,
{
fn parameters(&self) -> &'static [Parameter] {
ApiMethodInfo::parameters(self)
}
fn call(&self, params: Value) -> super::ApiFuture<Bytes> {
//async fn real_handler(this: &Self, params: Value) -> ApiOutput<Bytes> {
// (Self::handler(this))(params)
// .await
// .map(|res| res.into())
//}
let handler = self.handler();
use futures::future::TryFutureExt;
Box::pin(handler(params).map_ok(|res| res.map(|body| body.into())))
}
}
/// A reference to an API method. Note that when coming from the command line, it is possible to /// A reference to an API method. Note that when coming from the command line, it is possible to
/// match some parameters as positional parameters rather than argument switches, therefor this /// match some parameters as positional parameters rather than argument switches, therefor this
/// contains an ordered list of positional parameters. /// contains an ordered list of positional parameters.
@ -151,28 +183,22 @@ impl<Body: 'static> SubCommands<Body> {
/// Note that we currently do not support optional positional parameters. /// Note that we currently do not support optional positional parameters.
// XXX: If we want optional positional parameters - should we make an enum or just say the // XXX: If we want optional positional parameters - should we make an enum or just say the
// parameter name should have brackets around it? // parameter name should have brackets around it?
pub struct Method<Body: 'static> { pub struct Method {
pub method: MethodInfoRef<Body>, pub method: MethodInfoRef,
pub positional_args: &'static [&'static str], pub positional_args: &'static [&'static str],
//pub formatter: Option<()>, // TODO: output formatter //pub formatter: Option<()>, // TODO: output formatter
} }
impl<Body: 'static> Method<Body> { impl Method {
/// Create a new reference to an API method. /// Create a new reference to an API method.
pub fn new( pub fn new(method: MethodInfoRef, positional_args: &'static [&'static str]) -> Self {
method: &'static (dyn ApiMethodInfo<Body> + Send + Sync),
positional_args: &'static [&'static str],
) -> Self {
Self { Self {
method, method,
positional_args, positional_args,
} }
} }
fn resolve( fn resolve(&self, mut args: std::slice::Iter<&str>) -> Result<(MethodInfoRef, Value), Error> {
&self,
mut args: std::slice::Iter<&str>,
) -> Result<(MethodInfoRef<Body>, Value), Error> {
let mut params = serde_json::Map::new(); let mut params = serde_json::Map::new();
let mut positionals = self.positional_args.iter(); let mut positionals = self.positional_args.iter();

16
proxmox-api/src/router.rs
View File

@ -32,16 +32,16 @@ pub enum SubRoute<Body: 'static> {
#[derive(Default)] #[derive(Default)]
pub struct Router<Body: 'static> { pub struct Router<Body: 'static> {
/// The `GET` http method. /// The `GET` http method.
pub get: Option<&'static (dyn ApiMethodInfo<Body> + Send + Sync)>, pub get: Option<&'static (dyn ApiMethodInfo<Body = Body> + Send + Sync)>,
/// The `PUT` http method. /// The `PUT` http method.
pub put: Option<&'static (dyn ApiMethodInfo<Body> + Send + Sync)>, pub put: Option<&'static (dyn ApiMethodInfo<Body = Body> + Send + Sync)>,
/// The `POST` http method. /// The `POST` http method.
pub post: Option<&'static (dyn ApiMethodInfo<Body> + Send + Sync)>, pub post: Option<&'static (dyn ApiMethodInfo<Body = Body> + Send + Sync)>,
/// The `DELETE` http method. /// The `DELETE` http method.
pub delete: Option<&'static (dyn ApiMethodInfo<Body> + Send + Sync)>, pub delete: Option<&'static (dyn ApiMethodInfo<Body = Body> + Send + Sync)>,
/// Specifies the behavior of sub directories. See [`SubRoute`]. /// Specifies the behavior of sub directories. See [`SubRoute`].
pub subroute: Option<SubRoute<Body>>, pub subroute: Option<SubRoute<Body>>,
@ -165,7 +165,7 @@ where
/// Builder method to provide a `GET` method info. /// Builder method to provide a `GET` method info.
pub fn get<I>(mut self, method: &'static I) -> Self pub fn get<I>(mut self, method: &'static I) -> Self
where where
I: ApiMethodInfo<Body> + Send + Sync, I: ApiMethodInfo<Body = Body> + Send + Sync,
{ {
self.get = Some(method); self.get = Some(method);
self self
@ -174,7 +174,7 @@ where
/// Builder method to provide a `PUT` method info. /// Builder method to provide a `PUT` method info.
pub fn put<I>(mut self, method: &'static I) -> Self pub fn put<I>(mut self, method: &'static I) -> Self
where where
I: ApiMethodInfo<Body> + Send + Sync, I: ApiMethodInfo<Body = Body> + Send + Sync,
{ {
self.put = Some(method); self.put = Some(method);
self self
@ -183,7 +183,7 @@ where
/// Builder method to provide a `POST` method info. /// Builder method to provide a `POST` method info.
pub fn post<I>(mut self, method: &'static I) -> Self pub fn post<I>(mut self, method: &'static I) -> Self
where where
I: ApiMethodInfo<Body> + Send + Sync, I: ApiMethodInfo<Body = Body> + Send + Sync,
{ {
self.post = Some(method); self.post = Some(method);
self self
@ -192,7 +192,7 @@ where
/// Builder method to provide a `DELETE` method info. /// Builder method to provide a `DELETE` method info.
pub fn delete<I>(mut self, method: &'static I) -> Self pub fn delete<I>(mut self, method: &'static I) -> Self
where where
I: ApiMethodInfo<Body> + Send + Sync, I: ApiMethodInfo<Body = Body> + Send + Sync,
{ {
self.delete = Some(method); self.delete = Some(method);
self self

2
proxmox-api/tests/cli.rs
View File

@ -65,7 +65,7 @@ fn simple() {
); );
} }
fn check_cli(cli: &cli::App<Bytes>, args: &[&str], expect: Result<&str, &str>) { fn check_cli(cli: &cli::App, args: &[&str], expect: Result<&str, &str>) {
match (cli.run(args), expect) { match (cli.run(args), expect) {
(Ok(result), Ok(expect)) => { (Ok(result), Ok(expect)) => {
let body = std::str::from_utf8(result.body().as_ref()) let body = std::str::from_utf8(result.body().as_ref())