From bd79dd8f021553eb537ecc9c8355823f945fc456 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Wed, 19 Jun 2019 15:16:40 +0200 Subject: [PATCH] api: make method body an associated type 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. 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 --- proxmox-api-macro/src/api_macro.rs | 4 +- proxmox-api/Cargo.toml | 2 +- proxmox-api/src/api_type.rs | 12 ++-- proxmox-api/src/cli.rs | 102 ++++++++++++++++++----------- proxmox-api/src/router.rs | 16 ++--- proxmox-api/tests/cli.rs | 2 +- 6 files changed, 85 insertions(+), 53 deletions(-) diff --git a/proxmox-api-macro/src/api_macro.rs b/proxmox-api-macro/src/api_macro.rs index 9d0ae60b..166d165f 100644 --- a/proxmox-api-macro/src/api_macro.rs +++ b/proxmox-api-macro/src/api_macro.rs @@ -341,7 +341,9 @@ fn handle_function( // // Note that technically we don't need the `description` member in this trait, as this is // 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_api_description } diff --git a/proxmox-api/Cargo.toml b/proxmox-api/Cargo.toml index b9e2cf32..59160b17 100644 --- a/proxmox-api/Cargo.toml +++ b/proxmox-api/Cargo.toml @@ -5,6 +5,7 @@ version = "0.1.0" authors = [ "Wolfgang Bumiller " ] [dependencies] +bytes = "0.4" failure = "0.1" futures-preview = "0.3.0-alpha" http = "0.1" @@ -13,5 +14,4 @@ serde_derive = "1.0" serde_json = "1.0" [dev-dependencies] -bytes = "0.4" lazy_static = "1.3" diff --git a/proxmox-api/src/api_type.rs b/proxmox-api/src/api_type.rs index 7e064824..63a450f8 100644 --- a/proxmox-api/src/api_type.rs +++ b/proxmox-api/src/api_type.rs @@ -7,13 +7,15 @@ use serde_json::{json, Value}; /// 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 /// a method. -pub trait ApiMethodInfo { +pub trait ApiMethodInfo { + type Body; + fn description(&self) -> &'static str; fn parameters(&self) -> &'static [Parameter]; fn return_type(&self) -> &'static TypeInfo; fn protected(&self) -> bool; fn reload_timezone(&self) -> bool; - fn handler(&self) -> fn(Value) -> super::ApiFuture; + fn handler(&self) -> fn(Value) -> super::ApiFuture; } /// 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 { pub handler: fn(Value) -> super::ApiFuture, } -impl ApiMethodInfo for ApiMethod { +impl ApiMethodInfo for ApiMethod { + type Body = Body; + fn description(&self) -> &'static str { self.description } @@ -110,7 +114,7 @@ impl ApiMethodInfo for ApiMethod { } } -impl dyn ApiMethodInfo + Send + Sync { +impl dyn ApiMethodInfo + Send + Sync { pub fn api_dump(&self) -> Value { let parameters = Value::Object(std::iter::FromIterator::from_iter( self.parameters() diff --git a/proxmox-api/src/cli.rs b/proxmox-api/src/cli.rs index 5fefc9a8..4f12a52c 100644 --- a/proxmox-api/src/cli.rs +++ b/proxmox-api/src/cli.rs @@ -3,21 +3,22 @@ use std::collections::HashMap; use std::str::FromStr; +use bytes::Bytes; use failure::{bail, format_err, Error}; use serde::Serialize; use serde_json::Value; use super::{ApiMethodInfo, ApiOutput, Parameter}; -type MethodInfoRef = &'static (dyn ApiMethodInfo + Send + Sync); +type MethodInfoRef = &'static dyn UnifiedApiMethod; /// A CLI root node. -pub struct App { +pub struct App { name: &'static str, - command: Option>, + command: Option, } -impl App { +impl App { /// Create a new empty App instance. pub fn new(name: &'static str) -> Self { Self { @@ -29,7 +30,7 @@ impl App { /// Directly connect this instance to a single API method. /// /// This is a builder method and will panic if there's already a method registered! - pub fn method(mut self, method: Method) -> Self { + pub fn method(mut self, method: Method) -> Self { assert!( self.command.is_none(), "app {} already has a comman!", @@ -44,7 +45,7 @@ impl App { /// /// This is a builder method and will panic if the subcommand already exists or no subcommands /// may be added. - pub fn subcommand(mut self, name: &'static str, subcommand: Command) -> Self { + pub fn subcommand(mut self, name: &'static str, subcommand: Command) -> Self { match self .command .get_or_insert_with(|| Command::SubCommands(SubCommands::new())) @@ -58,7 +59,7 @@ impl App { } /// Resolve a list of parameters to a method and a parameter json value. - pub fn resolve(&self, args: &[&str]) -> Result<(MethodInfoRef, Value), Error> { + pub fn resolve(&self, args: &[&str]) -> Result<(MethodInfoRef, Value), Error> { self.command .as_ref() .ok_or_else(|| format_err!("no commands available"))? @@ -66,25 +67,29 @@ impl App { } /// Run a command through this command line interface. - pub fn run(&self, args: &[&str]) -> ApiOutput { + pub fn run(&self, args: &[&str]) -> ApiOutput { let (method, params) = self.resolve(args)?; - let handler = method.handler(); - futures::executor::block_on(handler(params)) + let future = method.call(params); + futures::executor::block_on(future) } } /// A node in the CLI command router. This is either -pub enum Command { - Method(Method), - SubCommands(SubCommands), +pub enum Command { + Method(Method), + SubCommands(SubCommands), } -impl Command { +impl Command { /// Create a Command entry pointing to an API method - pub fn method( - method: &'static (dyn ApiMethodInfo + Send + Sync), + pub fn method( + method: &'static T, positional_args: &'static [&'static str], - ) -> Self { + ) -> Self + where + T: ApiMethodInfo, + T::Body: 'static + Into, + { Command::Method(Method::new(method, positional_args)) } @@ -93,7 +98,7 @@ impl Command { Command::SubCommands(SubCommands::new()) } - fn resolve(&self, args: std::slice::Iter<&str>) -> Result<(MethodInfoRef, Value), Error> { + fn resolve(&self, args: std::slice::Iter<&str>) -> Result<(MethodInfoRef, Value), Error> { match self { Command::Method(method) => method.resolve(args), Command::SubCommands(subcmd) => subcmd.resolve(args), @@ -101,11 +106,11 @@ impl Command { } } -pub struct SubCommands { - commands: HashMap<&'static str, Command>, +pub struct SubCommands { + commands: HashMap<&'static str, Command>, } -impl SubCommands { +impl SubCommands { /// Create a new empty SubCommands hash. pub fn new() -> Self { Self { @@ -116,7 +121,7 @@ impl SubCommands { /// Add a subcommand. /// /// 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) -> &mut Self { + pub fn add_subcommand(&mut self, name: &'static str, command: Command) -> &mut Self { let old = self.commands.insert(name, command); assert!(old.is_none(), "subcommand '{}' already exists", name); self @@ -125,15 +130,12 @@ impl SubCommands { /// Builder method to add a subcommand. /// /// 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) -> Self { + pub fn subcommand(mut self, name: &'static str, command: Command) -> Self { self.add_subcommand(name, command); self } - fn resolve( - &self, - mut args: std::slice::Iter<&str>, - ) -> Result<(MethodInfoRef, Value), Error> { + fn resolve(&self, mut args: std::slice::Iter<&str>) -> Result<(MethodInfoRef, Value), Error> { match args.next() { None => bail!("missing subcommand"), Some(arg) => match self.commands.get(arg) { @@ -144,6 +146,36 @@ impl SubCommands { } } +/// 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`. +pub trait UnifiedApiMethod: Send + Sync { + fn parameters(&self) -> &'static [Parameter]; + fn call(&self, params: Value) -> super::ApiFuture; +} + +impl UnifiedApiMethod for T +where + T: ApiMethodInfo, + T::Body: 'static + Into, +{ + fn parameters(&self) -> &'static [Parameter] { + ApiMethodInfo::parameters(self) + } + + fn call(&self, params: Value) -> super::ApiFuture { + //async fn real_handler(this: &Self, params: Value) -> ApiOutput { + // (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 /// match some parameters as positional parameters rather than argument switches, therefor this /// contains an ordered list of positional parameters. @@ -151,28 +183,22 @@ impl SubCommands { /// 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 // parameter name should have brackets around it? -pub struct Method { - pub method: MethodInfoRef, +pub struct Method { + pub method: MethodInfoRef, pub positional_args: &'static [&'static str], //pub formatter: Option<()>, // TODO: output formatter } -impl Method { +impl Method { /// Create a new reference to an API method. - pub fn new( - method: &'static (dyn ApiMethodInfo + Send + Sync), - positional_args: &'static [&'static str], - ) -> Self { + pub fn new(method: MethodInfoRef, positional_args: &'static [&'static str]) -> Self { Self { method, positional_args, } } - fn resolve( - &self, - mut args: std::slice::Iter<&str>, - ) -> Result<(MethodInfoRef, Value), Error> { + fn resolve(&self, mut args: std::slice::Iter<&str>) -> Result<(MethodInfoRef, Value), Error> { let mut params = serde_json::Map::new(); let mut positionals = self.positional_args.iter(); diff --git a/proxmox-api/src/router.rs b/proxmox-api/src/router.rs index 0e1e00b8..33c3d1cf 100644 --- a/proxmox-api/src/router.rs +++ b/proxmox-api/src/router.rs @@ -32,16 +32,16 @@ pub enum SubRoute { #[derive(Default)] pub struct Router { /// The `GET` http method. - pub get: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, + pub get: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, /// The `PUT` http method. - pub put: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, + pub put: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, /// The `POST` http method. - pub post: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, + pub post: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, /// The `DELETE` http method. - pub delete: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, + pub delete: Option<&'static (dyn ApiMethodInfo + Send + Sync)>, /// Specifies the behavior of sub directories. See [`SubRoute`]. pub subroute: Option>, @@ -165,7 +165,7 @@ where /// Builder method to provide a `GET` method info. pub fn get(mut self, method: &'static I) -> Self where - I: ApiMethodInfo + Send + Sync, + I: ApiMethodInfo + Send + Sync, { self.get = Some(method); self @@ -174,7 +174,7 @@ where /// Builder method to provide a `PUT` method info. pub fn put(mut self, method: &'static I) -> Self where - I: ApiMethodInfo + Send + Sync, + I: ApiMethodInfo + Send + Sync, { self.put = Some(method); self @@ -183,7 +183,7 @@ where /// Builder method to provide a `POST` method info. pub fn post(mut self, method: &'static I) -> Self where - I: ApiMethodInfo + Send + Sync, + I: ApiMethodInfo + Send + Sync, { self.post = Some(method); self @@ -192,7 +192,7 @@ where /// Builder method to provide a `DELETE` method info. pub fn delete(mut self, method: &'static I) -> Self where - I: ApiMethodInfo + Send + Sync, + I: ApiMethodInfo + Send + Sync, { self.delete = Some(method); self diff --git a/proxmox-api/tests/cli.rs b/proxmox-api/tests/cli.rs index 65b1d7c2..5a758bb0 100644 --- a/proxmox-api/tests/cli.rs +++ b/proxmox-api/tests/cli.rs @@ -65,7 +65,7 @@ fn simple() { ); } -fn check_cli(cli: &cli::App, args: &[&str], expect: Result<&str, &str>) { +fn check_cli(cli: &cli::App, args: &[&str], expect: Result<&str, &str>) { match (cli.run(args), expect) { (Ok(result), Ok(expect)) => { let body = std::str::from_utf8(result.body().as_ref())