[rpc module] test helper for calling and converting types to JSON-RPC params #458
Conversation
…nrpsee into na-rpc-module-test-helper
Co-authored-by: David <dvdplm@gmail.com>
utils/src/server/rpc_module.rs
Outdated
| /// 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> { | ||
| let params = serde_json::to_string(¶ms).ok().map(|json| { | ||
| let json = if json.starts_with('[') && json.ends_with(']') { json } else { format!("[{}]", json) }; |
There was a problem hiding this comment.
What if there's only one param provided to the call, and it's something that already serializes to an array like Vec<u8>?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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]]
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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")
There was a problem hiding this comment.
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! :))
There was a problem hiding this comment.
that's good idea, I think that's better
There was a problem hiding this comment.
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. :)
|
|
||
| impl<P: Serialize> ToRpcParams for Vec<P> {} | ||
|
|
||
| macro_rules! array_impls { |
There was a problem hiding this comment.
const generics doesn't work because of serde I think.
There was a problem hiding this comment.
Ah, came here to ask about this.
|
I'll merge it so @dvdplm can continue with his work :) |
Follow-up on #443
A little bit hacky to do like this but I think it will work in order to avoid users to manually serializing their parameters to
serde::RawValueand manually add it to be serialized to a sequence"[ .... ]"to work properly if it's not already the case.See examples in the introduced tests