From 72ed48961199da45e4bae472060ab7de09dbe291 Mon Sep 17 00:00:00 2001 From: Fabio Valentini Date: Wed, 5 Jul 2023 16:35:12 +0200 Subject: [PATCH] server: first draft of system.multicall support --- Cargo.toml | 1 + README.md | 27 ++++- checkall.py | 6 +- dxr/Cargo.toml | 5 + dxr/src/error.rs | 2 +- dxr/src/lib.rs | 2 + dxr/src/multicall.rs | 162 +++++++++++++++++++++++++++++- dxr_client/Cargo.toml | 3 + dxr_client/README.md | 2 + dxr_client/src/call.rs | 8 +- dxr_client/src/reqwest_support.rs | 18 ++-- dxr_server/Cargo.toml | 4 + dxr_server/README.md | 2 + dxr_server/src/lib.rs | 40 +++++++- dxr_tests/Cargo.toml | 6 +- dxr_tests/tests/adder.rs | 21 +++- 16 files changed, 285 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 96cb2de..43052b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ members = [ "dxr_server", "dxr_tests", ] +resolver = "2" [workspace.package] version = "0.6.0-dev" diff --git a/README.md b/README.md index ba0c529..1f3732f 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,32 @@ returns the number of times the `countme()` method has been called since the ser The `dxr` crate provides functionality for deriving the `TryFromDXR` and `TryToDXR` traits if the `derive` feature is enabled. -There is also optional support for two common, non-standard XML-RPC extensions: +There is also optional support for common, non-standard XML-RPC extensions: - "long" 64-bit integers (``): mapped to `i64`, enabled with the `i8` feature - "null" values (``): mapped to `Option`, enabled with the `nil` feature +- "system.multicall" support for processing multiple RPC calls within a single request, + enabled with the `multicall` feature + +## Development + +This repository contains two helper scripts for helping with development: + +- `./checkall.py`: Runs `cargo check`, `cargo clippy`, and `cargo test` for all crates and all + combinations of enabled optional features. There should be no warnings or errors for any + combination of enabled features. When adding new feature to one of the crates, the list of + features of each crate needs to be updated in this script as well. +- `./coverage.sh`: Builds the crate with coverage instrumentation enabled, runs + `cargo test --workspace --all-features`, and generates a test coverage report with `grcov`, which + can then be viewed in `target/debug/coverage/`. + +## License + +This project is licensed under either of + + * Apache License, Version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or + https://www.apache.org/licenses/LICENSE-2.0) + * MIT license ([LICENSE-MIT](LICENSE-MIT) or + https://opensource.org/licenses/MIT) + +at your option. diff --git a/checkall.py b/checkall.py index b4a125d..e8bec2a 100755 --- a/checkall.py +++ b/checkall.py @@ -45,10 +45,10 @@ def check(package: str, feature_list: list[str]): def main(): os.environ["QUICKCHECK_TESTS"] = "100000" - check("dxr", ["derive", "i8", "nil"]) + check("dxr", ["derive", "multicall", "i8", "nil"]) check("dxr_derive", []) - check("dxr_client", ["default", "reqwest", "default-tls", "native-tls", "rustls-tls"]) - check("dxr_server", ["default", "axum"]) + check("dxr_client", ["default", "multicall", "reqwest", "default-tls", "native-tls", "rustls-tls"]) + check("dxr_server", ["default", "multicall", "axum"]) check("dxr_tests", []) diff --git a/dxr/Cargo.toml b/dxr/Cargo.toml index 0fed12f..614f7a1 100644 --- a/dxr/Cargo.toml +++ b/dxr/Cargo.toml @@ -34,8 +34,13 @@ trybuild = "1" [features] # support for derive macros derive =["dep:dxr_derive"] + +# utilities for multicall support +multicall = [] + # support non-standard XML-RPC values i8 = [] + # support non-standard XML-RPC values nil = [] diff --git a/dxr/src/error.rs b/dxr/src/error.rs index 7e2ae01..b1cb96b 100644 --- a/dxr/src/error.rs +++ b/dxr/src/error.rs @@ -4,7 +4,7 @@ use thiserror::Error; use crate::fault::Fault; -#[derive(Debug, Error)] +#[derive(Debug, Error, PartialEq)] /// Error type representing conversion errors between XML-RPC values and Rust values. pub enum DxrError { /// Error variant for XML parser errors. diff --git a/dxr/src/lib.rs b/dxr/src/lib.rs index ea1793f..9ad4c5e 100644 --- a/dxr/src/lib.rs +++ b/dxr/src/lib.rs @@ -72,7 +72,9 @@ pub use fault::*; mod impls; +#[cfg(feature = "multicall")] mod multicall; +#[cfg(feature = "multicall")] pub use multicall::*; mod traits; diff --git a/dxr/src/multicall.rs b/dxr/src/multicall.rs index 0d02313..82446f4 100644 --- a/dxr/src/multicall.rs +++ b/dxr/src/multicall.rs @@ -1,11 +1,13 @@ -use crate::{DxrError, Member, Struct, TryToParams, TryToValue, Value}; +use std::collections::HashMap; + +use crate::{Array, DxrError, Fault, Member, Struct, TryFromValue, TryToParams, TryToValue, Value}; /// Convenience method for constructing arguments for "system.multicall" calls. /// /// This method is more efficient than manually constructing the array of XML-RPC /// calls as structs (either by using a [`HashMap`] or by deriving [`TryToValue`] /// on a custom struct) because this method can rely on crate internals. -pub fn multicall

(calls: Vec<(String, P)>) -> Result +pub fn into_multicall_params

(calls: Vec<(String, P)>) -> Result where P: TryToParams, { @@ -22,3 +24,159 @@ where params.try_to_value() } + +/// Convenience method for reconstructing method calls from "system.multicall" arguments. +#[allow(clippy::type_complexity)] +pub fn from_multicall_params(mut values: Vec) -> Result), DxrError>>, DxrError> { + // system.multicall calls take an array of arguments as single argument + let value = match values.pop() { + Some(value) => value, + None => return Err(DxrError::parameter_mismatch(0, 1)), + }; + + // check if there are more than one arguments + if !values.is_empty() { + return Err(DxrError::parameter_mismatch(values.len() + 1, 1)); + } + + // extract vector of argument values + let params = >::try_from_value(&value)?; + + let calls: Vec), DxrError>> = params + .into_iter() + .map(|v| { + let mut members: HashMap = HashMap::try_from_value(&v)?; + + if members.len() != 2 { + return Err(DxrError::parameter_mismatch(members.len(), 2)); + } + + let name = match members.remove("methodName") { + Some(name) => name, + None => return Err(DxrError::missing_field("system.multicall", "methodName")), + }; + + let params = match members.remove("params") { + Some(params) => params, + None => return Err(DxrError::missing_field("system.multicall", "params")), + }; + + Ok((String::try_from_value(&name)?, >::try_from_value(¶ms)?)) + }) + .collect(); + + Ok(calls) +} + +/// Convenience method for constructing return values for "system.multicall" calls. +pub fn into_multicall_response(results: Vec>) -> Value { + let values: Vec = results + .into_iter() + .map(|r| match r { + Ok(value) => Value::array(Array::new(vec![value])), + Err(fault) => { + let members = vec![ + Member::new(String::from("faultCode"), Value::i4(fault.code())), + Member::new(String::from("faultString"), Value::string(fault.string().to_owned())), + ]; + Value::structure(Struct::new(members)) + }, + }) + .collect(); + + Value::array(Array::new(values)) +} + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + use super::*; + use crate::MethodCall; + + #[test] + fn from_multicall() { + let string = "\ + + system.multicall + + + + + + + + + methodName + event + + + params + + + + foo + bar + baz + 1 + + + + + + + + + + methodName + event + + + params + + + + another + call + hi + 1 + + + + + + + + + + + +"; + + let call: MethodCall = quick_xml::de::from_str(string).unwrap(); + + let params = from_multicall_params(call.params()).unwrap(); + + let expected: Vec), DxrError>> = vec![ + Ok(( + String::from("event"), + vec![ + Value::string(String::from("foo")), + Value::string(String::from("bar")), + Value::string(String::from("baz")), + Value::boolean(true), + ], + )), + Ok(( + String::from("event"), + vec![ + Value::string(String::from("another")), + Value::string(String::from("call")), + Value::string(String::from("hi")), + Value::boolean(true), + ], + )), + ]; + + assert_eq!(params, expected); + } +} diff --git a/dxr_client/Cargo.toml b/dxr_client/Cargo.toml index 31b6ace..87a1580 100644 --- a/dxr_client/Cargo.toml +++ b/dxr_client/Cargo.toml @@ -26,6 +26,9 @@ url = { version = "2.2", optional = true } # use the default TLS backend by default default = ["reqwest?/default-tls"] +# multicall support +multicall = ["dxr/multicall"] + reqwest = ["dep:http", "dep:reqwest", "dep:thiserror", "dep:url"] default-tls = ["reqwest?/default-tls"] diff --git a/dxr_client/README.md b/dxr_client/README.md index 9101bee..05a4466 100644 --- a/dxr_client/README.md +++ b/dxr_client/README.md @@ -11,3 +11,5 @@ This crate contains a building blocks for writing XML-RPC clients based on `dxr` It also includes an implementation of an `async` XML-RPC client using `reqwest`, which is disabled by default. To enable the `reqwest` support, enable the `"reqwest"` feature of this crate. + +To enable convenience functionality for "system.multicall" support, enable the `multicall` feature. diff --git a/dxr_client/src/call.rs b/dxr_client/src/call.rs index 4cd5a41..766dfd7 100644 --- a/dxr_client/src/call.rs +++ b/dxr_client/src/call.rs @@ -58,14 +58,14 @@ where } } +#[cfg(feature = "multicall")] impl

Call<'static, P, Vec> where P: TryToParams, { /// Constructor for [`Call`] values for `system.multicall` calls. - // FIXME: adapt type of return value: `Vec>` pub fn multicall(calls: Vec<(String, P)>) -> Result>, DxrError> { - let calls = dxr::multicall(calls)?; + let calls = dxr::into_multicall_params(calls)?; Ok(Call::new("system.multicall", calls)) } } @@ -73,8 +73,11 @@ where #[cfg(test)] mod tests { #![allow(clippy::unwrap_used)] + + #[cfg(feature = "multicall")] use super::*; + #[cfg(feature = "multicall")] #[test] fn to_multicall() { let call = Call::multicall(vec![(String::from("add"), (1, 2)), (String::from("sub"), (2, 1))]).unwrap(); @@ -109,6 +112,7 @@ mod tests { ".replace('\n', ""); + assert_eq!(string, expected); } } diff --git a/dxr_client/src/reqwest_support.rs b/dxr_client/src/reqwest_support.rs index 57d0806..200195c 100644 --- a/dxr_client/src/reqwest_support.rs +++ b/dxr_client/src/reqwest_support.rs @@ -1,10 +1,13 @@ +#[cfg(feature = "multicall")] use std::collections::HashMap; use http::header::{HeaderMap, HeaderName, HeaderValue, CONTENT_TYPE, USER_AGENT}; use thiserror::Error; use url::Url; -use dxr::{DxrError, Fault, FaultResponse, MethodCall, MethodResponse, TryFromValue, TryToParams, Value}; +#[cfg(feature = "multicall")] +use dxr::Value; +use dxr::{DxrError, Fault, FaultResponse, MethodCall, MethodResponse, TryFromValue, TryToParams}; use crate::{Call, DEFAULT_USER_AGENT}; @@ -132,20 +135,16 @@ impl Client { } /// Asynchronous method for handling "system.multicall" calls. + /// + /// *Note*: This method does not check if the number of method calls matches the number of + /// returned results. + #[cfg(feature = "multicall")] pub async fn multicall( &self, call: Call<'_, P, Vec>, ) -> Result>, ClientError> { - let expected = call.params()?.len(); let response = self.call(call).await?; - if response.len() != expected { - // length of results must match number of method calls - return Err(ClientError::RPC { - error: DxrError::parameter_mismatch(response.len(), expected), - }); - } - let mut results = Vec::new(); for result in response { // return values for successful calls are arrays that contain a single value @@ -178,7 +177,6 @@ impl Client { } } - fn request_to_body(call: &MethodCall) -> Result { let body = [ r#""#, diff --git a/dxr_server/Cargo.toml b/dxr_server/Cargo.toml index 276b0ba..20f0bdf 100644 --- a/dxr_server/Cargo.toml +++ b/dxr_server/Cargo.toml @@ -25,6 +25,10 @@ tokio = { version = "1.14", features = ["sync"], optional = true } [features] default = [] + +# multicall support +multicall = ["dxr/multicall"] + axum = ["dep:axum", "dep:hyper", "dep:thiserror", "dep:tokio"] [package.metadata.docs.rs] diff --git a/dxr_server/README.md b/dxr_server/README.md index c8d2a07..19c5701 100644 --- a/dxr_server/README.md +++ b/dxr_server/README.md @@ -11,3 +11,5 @@ This crate contains a building blocks for writing XML-RPC servers based on `dxr` It also includes a complete XML-RPC server implementation based on the `axum` web framework, which is disabled by default. To enable the `axum` support, enable the `"axum"` feature of this crate. + +To enable "system.multicall" support, enable the `multicall` feature. diff --git a/dxr_server/src/lib.rs b/dxr_server/src/lib.rs index a52aee9..4dd1971 100644 --- a/dxr_server/src/lib.rs +++ b/dxr_server/src/lib.rs @@ -58,11 +58,49 @@ pub async fn server(handlers: HandlerMap, body: &str, headers: HeaderMap) -> (St Ok(call) => call, Err(error) => { let e = DxrError::invalid_data(error.to_string()); - let f = Fault::new(400, e.to_string()); + let f = Fault::from(e); return fault_to_response(f.code(), f.string()); }, }; + #[cfg(feature = "multicall")] + if call.name() == "system.multicall" { + let calls = match dxr::from_multicall_params(call.params()) { + Ok(calls) => calls, + Err(error) => { + let f = Fault::from(error); + return fault_to_response(f.code(), f.string()); + }, + }; + + let mut results = Vec::new(); + + for multi in calls { + match multi { + Ok((name, params)) => { + let handler = match handlers.get(name.as_str()) { + Some(handler) => handler, + None => { + results.push(Err(Fault::new(404, String::from("Unknown method.")))); + continue; + }, + }; + + let result = handler.handle(¶ms, headers.clone()).await; + results.push(result); + }, + Err(error) => { + results.push(Err(Fault::from(error))); + continue; + }, + } + } + + let value = dxr::into_multicall_response(results); + + return success_to_response(value); + } + let handler = match handlers.get(call.name()) { Some(handler) => handler, None => return fault_to_response(404, "Unknown method."), diff --git a/dxr_tests/Cargo.toml b/dxr_tests/Cargo.toml index 74c7b44..38c2407 100644 --- a/dxr_tests/Cargo.toml +++ b/dxr_tests/Cargo.toml @@ -38,10 +38,10 @@ path = "tests/echo_one.rs" [dependencies] [dev-dependencies] -dxr = { workspace = true, features = ["derive", "i8", "nil"] } +dxr = { workspace = true, features = ["derive", "multicall", "i8", "nil"] } dxr_derive.workspace = true -dxr_client = { workspace = true, features = ["reqwest"] } -dxr_server = { workspace = true, features = ["axum"] } +dxr_client = { workspace = true, features = ["multicall", "reqwest"] } +dxr_server = { workspace = true, features = ["multicall", "axum"] } chrono = { version = "0.4.19", features = ["clock"], default-features = false } tokio = { version = "1.14", features = ["macros", "rt-multi-thread", "time"] } diff --git a/dxr_tests/tests/adder.rs b/dxr_tests/tests/adder.rs index e53623a..8f5e14f 100644 --- a/dxr_tests/tests/adder.rs +++ b/dxr_tests/tests/adder.rs @@ -2,7 +2,7 @@ use std::time::Duration; -use dxr::{TryFromParams, TryToValue, Value}; +use dxr::{Fault, TryFromParams, TryToValue, Value}; use dxr_client::{Call, ClientBuilder, ClientError}; use dxr_server::{axum::http::HeaderMap, HandlerFn, HandlerResult, RouteBuilder, Server}; @@ -53,6 +53,25 @@ async fn adder() { let r: i32 = client.call(call).await.unwrap(); assert_eq!((a + b), r); + // multicall + let call = Call::multicall(vec![ + (String::from("add"), (1, 2)), + (String::from("add"), (-3, -5)), + (String::from("add"), (-1, 1)), + (String::from("sub"), (1, 2)), + ]) + .unwrap(); + let values = client.multicall(call).await.unwrap(); + assert_eq!( + values, + vec![ + Ok(Value::i4(3)), + Ok(Value::i4(-8)), + Ok(Value::i4(0)), + Err(Fault::new(404, String::from("Unknown method."))) + ] + ); + // argument number mismatch let (a, b, c) = (2i32, 3i32, 4i32); let call: Call<_, i32> = Call::new("add", (a, b, c));