[RFC] impl ToParams for json::Value

[vincenzopalazzo]

I am having a problem passing a json object to my rpc server.

This PR is starting the work to allow a {...} as a Params. Suggestions on how to improve this will be super welcome.

The related part of the spec is https://www.jsonrpc.org/specification

4.2 Parameter Structures
If present, parameters for the rpc call MUST be provided as a Structured value. Either by-position through an Array or by-name through an Object.

by-position: params MUST be an Array, containing the values in the Server expected order.
by-name: params MUST be an Object, with member names that match the Server expected parameter names. The absence of expected names MAY result in an error being generated. The names MUST match exactly, including case, to the method's expected parameters.

[niklasad1]

Looks good to me but I think we need to add serde_json::value::RawValue as well then.

Can be done in this PR or another one...

[vincenzopalazzo]

Can be done in this PR or another one...

I can do it in another PR if you like

[niklasad1]

/cc @jsdw maybe it does more sense to implement this for P: Serialize to be generic?

We already implement this for vec, slices and arrays....

[vincenzopalazzo]

I tried IIRC but there was an error, but yes I think this would be a really good improvment

[niklasad1]

After looking at this again, it's technically possible that the serde_json::Value isn't represented as an array or object.

For instance serde_json::Value::String("Foo") will be serialized to be "Foo" which isn't valid JSON-RPC params it must be array or object.

So, can you please write a specialized ToRpcParams impl instead of using the serializing decl macro?

I think we could do something similar to this:

impl ToRpcParams for serde_json::Value {
	fn to_rpc_params(self) -> Result<Option<Box<RawValue>>, serde_json::Error> {
		let json = match self {
			serde_json::Value::Array(a) => serde_json::to_string(&a),
			serde_json::Value::Object(o) => serde_json::to_string(&o),
			serde_json::Value::Null => Ok("[null]".to_string()),
			serde_json::Value::Bool(b) => serde_json::to_string(&[b]),
			serde_json::Value::String(s) => serde_json::to_string(&[s]),
			serde_json::Value::Number(n) => serde_json::to_string(&[n]),
		}?;

		RawValue::from_string(json).map(Some)
	}
}

[jsdw]

The only thing with Ok(None) iirc is that it's to indicate that no params have been provided, which might be confusing if some were. Really, I guess it should result in an error.

You could implement ToRpcParams on eg https://docs.rs/serde_json/latest/serde_json/struct.Map.html though, which should always be valid?

[niklasad1]

The only thing with Ok(None) iirc is that it's to indicate that no params have been provided, which might be confusing if some were. Really, I guess it should result in an error.

We could also just serialize it and leave it to user to deal with it but yeah Ok(None) is bad.

You could implement ToRpcParams on eg https://docs.rs/serde_json/latest/serde_json/struct.Map.html though, which should always be valid?

Yes

[niklasad1]

Ok, the conclusion is that we are okay to implement this for serde_json::Map but the implementation for serde_json::Value becomes to opinionated and better that users wrap it in a new type pattern implement ToRpcParams themselves.

wrong