Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[rpc module] test helper for calling and converting types to JSON-RPC params #458

Merged
merged 12 commits into from Sep 13, 2021
Merged

[rpc module] test helper for calling and converting types to JSON-RPC params #458

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e6d8030
new helper for testing
niklasad1 Sep 9, 2021
87695e3
fix features
niklasad1 Sep 9, 2021
1eb4d5b
call_with_params -> call_with
niklasad1 Sep 9, 2021
95332be
call_with_params -> call_with
niklasad1 Sep 9, 2021
45cddc0
Merge branch 'na-rpc-module-test-helper' of github.com:paritytech/jso…
niklasad1 Sep 9, 2021
7967bc3
Update utils/src/server/rpc_module.rs
niklasad1 Sep 10, 2021
0f96612
fix trait bound
niklasad1 Sep 10, 2021
c524e46
Merge branch 'na-rpc-module-test-helper' of github.com:paritytech/jso…
niklasad1 Sep 10, 2021
a8fb086
Update utils/src/server/rpc_module.rs
niklasad1 Sep 10, 2021
e8f44b1
call_with -> test_call
niklasad1 Sep 10, 2021
2e1e0b5
fix grumbles
niklasad1 Sep 13, 2021
0e366ea
fix nits
niklasad1 Sep 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions utils/Cargo.toml
View file
Open in desktop
Expand Up @@ -9,7 +9,6 @@ license = "MIT"
[dependencies]
beef = { version = "0.5.1", features = ["impl_serde"] }
thiserror = { version = "1", optional = true }
tokio = { version = "1", features = ["macros"], optional = true }
futures-channel = { version = "0.3.14", default-features = false, optional = true }
futures-util = { version = "0.3.14", default-features = false, optional = true }
hyper = { version = "0.14.10", default-features = false, features = ["stream"], optional = true }
Expand All @@ -35,8 +34,8 @@ server = [
"log",
"parking_lot",
"rand",
"tokio"
]

[dev-dependencies]
serde_json = "1.0"
tokio = { version = "1", features = ["macros", "rt"] }
70 changes: 70 additions & 0 deletions utils/src/server/rpc_module.rs
View file
Open in desktop
Expand Up @@ -180,6 +180,17 @@ impl Methods {
}
}

/// Test helper to call a method on the `RPC module` without having to spin a server up.
///
/// Converts the params to a stringified array for you if it's not already serialized to a sequence.
pub async fn test_call<T: Serialize>(&self, method: &str, params: T) -> Option<String> {
niklasad1 marked this conversation as resolved.
Show resolved Hide resolved
let params = serde_json::to_string(&params).ok().map(|json| {
let json = if json.starts_with('[') && json.ends_with(']') { json } else { format!("[{}]", json) };
Copy link
Collaborator

@jsdw jsdw Sep 13, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there's only one param provided to the call, and it's something that already serializes to an array like Vec<u8>?

Copy link
Member Author

@niklasad1 niklasad1 Sep 13, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would already be serialized to an array then we won' insert [ json ], see if expression above :)

However, it could be weird if you put in a Vec<_> and expect it to be serialized as [[my vec]], lemme double check I forgot if I added a test for this.

Copy link
Collaborator

@jsdw jsdw Sep 13, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the case I was picturing; if there's an RPC call that accepts 1 param, a Vec<u8>, then I guess I couldn't use test_call, because using it would lead to params like [1,2,3] being passed in instead of a single param like [[1,2,3]]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yepp, you are right.

I did it this way to work with both RpcParams::parse and RpcParams::Sequence::next(), but it won't work with doing weird things or that. I guess I was fooled because I used JsonValue in the test....

Don't remember if I had an issue with the parser to get this work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is an chicken and egg problem it will always be an edge-case for this; so no good solution AFAIU.

Perhaps, I should revisit to try to add into_rpc_test_module or something from because this essentially becomes a footgun unless we do if params == edge_case then panic!("not supported")

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A call like test_call could accept something like T: IntoRpcParams, which is implemented for a bunch of tuple sizes (and maybe even slices and arrays) like:

impl <A: Serialize> IntoRpcParams for (A,) { .. }
impl <A: Serialize, B: Serialize> IntoRpcParams for (A,B) { .. }
impl <const N: usize, A: Serialize> IntoRpcParams for [A;N] { .. }

That way, You could call eg .test_call("foo", (a, b, c)) or .test_call("foo", [a]) or .test_call("foo", (a,)) with low overhead?

(perhaps something like that exists in JsonRpsee already and I'm not being very helpful; I haven't checked! :))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's good idea, I think that's better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I played around with tuples on a branch on saturday and it worked fine up to a point and then didn't. I ended up having to call some permutations with a string and some with a tuple, in the end I abandoned it because it felt like the user would have a deeper understanding of what's going on than I felt was warranted.

That said, it might work out better for you. :)

RawValue::from_string(json).expect("valid JSON string above; qed")
});
self.call(method, params).await
}

/// Helper alternative to `execute`, useful for writing unit tests without having to spin
/// a server up.
pub async fn call(&self, method: &str, params: Option<Box<RawValue>>) -> Option<String> {
Expand Down Expand Up @@ -540,6 +551,8 @@ fn subscription_closed_err(sub_id: u64) -> Error {
#[cfg(test)]
mod tests {
use super::*;
use serde_json::json;

#[test]
fn rpc_modules_with_different_contexts_can_be_merged() {
let cx = Vec::<u8>::new();
Expand Down Expand Up @@ -574,4 +587,61 @@ mod tests {
assert!(module.method("hello_world").is_some());
assert!(module.method("hello_foobar").is_some());
}

#[tokio::test]
async fn calling_method_without_server() {
// Call sync method with no params
let mut module = RpcModule::new(());
module.register_method("boo", |_: RpcParams, _| Ok(String::from("boo!"))).unwrap();
let result = module.test_call("boo", None::<()>).await.unwrap();
assert_eq!(result.as_ref(), String::from(r#"{"jsonrpc":"2.0","result":"boo!","id":0}"#));

// Call sync method with params
module
.register_method("foo", |params, _| {
let n: u16 = params.one().expect("valid params please");
Ok(n * 2)
})
.unwrap();
let result = &module.test_call("foo", &3).await.unwrap();
assert_eq!(result.as_ref(), String::from(r#"{"jsonrpc":"2.0","result":6,"id":0}"#));
let result = &module.test_call("foo", &[3]).await.unwrap();
assert_eq!(result.as_ref(), String::from(r#"{"jsonrpc":"2.0","result":6,"id":0}"#));

// Call async method with params and context
struct MyContext;
impl MyContext {
fn roo(&self, things: Vec<u8>) -> u16 {
things.iter().sum::<u8>().into()
}
}
let mut module = RpcModule::new(MyContext);
module
.register_async_method("roo", |params, ctx| {
let ns: Vec<u8> = params.parse().expect("valid params please");
async move { Ok(ctx.roo(ns)) }.boxed()
})
.unwrap();

module
.register_async_method("many_args", |params, _ctx| {
let mut seq = params.sequence();

let one: Vec<usize> = seq.next().unwrap();
let two: String = seq.next().unwrap();
let three: usize = seq.optional_next().unwrap().unwrap_or(0);

let res = one.iter().sum::<usize>() + two.as_bytes().len() + three;

async move { Ok(res) }.boxed()
})
.unwrap();

let result = &module.test_call("roo", &[12, 13]).await.unwrap();
assert_eq!(result.as_ref(), String::from(r#"{"jsonrpc":"2.0","result":25,"id":0}"#));

let json = vec![json!([1, 3, 7]), json!("oooh")];
let result = &module.test_call("many_args", &json).await.unwrap();
assert_eq!(result.as_ref(), String::from(r#"{"jsonrpc":"2.0","result":15,"id":0}"#));
}
}