Unverified Commit 4e95f436 authored by Niklas Adolfsson's avatar Niklas Adolfsson Committed by GitHub
Browse files

[types]: ID type instead of serde_json::RawValue (#325)

* get started

* add additional test

* fix nits

* cargo fmt

* [types]: write some tests.

* [http server]: send empty response on notifs

* [http server]: fix tests

* [rpc module]: send subscription response

* Update types/src/v2/error.rs

* fix nits

* cargo fmt

* Update types/src/v2/params.rs

* remove needless clone

* remove dead code

* [types]: impl PartialEq for JsonErrorObject + test

* use beef::Cow

* Update http-server/src/tests.rs
parent 4f51e2f2
......@@ -2,11 +2,11 @@ use crate::traits::Client;
use crate::transport::HttpTransportClient;
use crate::v2::request::{JsonRpcCallSer, JsonRpcNotificationSer};
use crate::v2::{
error::JsonRpcErrorAlloc,
error::JsonRpcError,
params::{Id, JsonRpcParams},
response::JsonRpcResponse,
};
use crate::{Error, JsonRawValue, TEN_MB_SIZE_BYTES};
use crate::{Error, TEN_MB_SIZE_BYTES};
use async_trait::async_trait;
use fnv::FnvHashMap;
use serde::de::DeserializeOwned;
......@@ -76,12 +76,12 @@ impl Client for HttpClient {
let response: JsonRpcResponse<_> = match serde_json::from_slice(&body) {
Ok(response) => response,
Err(_) => {
let err: JsonRpcErrorAlloc = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err));
let err: JsonRpcError = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err.to_string()));
}
};
let response_id = parse_request_id(response.id)?;
let response_id = response.id.as_number().copied().ok_or(Error::InvalidRequestId)?;
if response_id == id {
Ok(response.result)
......@@ -115,15 +115,15 @@ impl Client for HttpClient {
let rps: Vec<JsonRpcResponse<_>> = match serde_json::from_slice(&body) {
Ok(response) => response,
Err(_) => {
let err: JsonRpcErrorAlloc = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err));
let err: JsonRpcError = serde_json::from_slice(&body).map_err(Error::ParseError)?;
return Err(Error::Request(err.to_string()));
}
};
// NOTE: `R::default` is placeholder and will be replaced in loop below.
let mut responses = vec![R::default(); ordered_requests.len()];
for rp in rps {
let response_id = parse_request_id(rp.id)?;
let response_id = rp.id.as_number().copied().ok_or(Error::InvalidRequestId)?;
let pos = match request_set.get(&response_id) {
Some(pos) => *pos,
None => return Err(Error::InvalidRequestId),
......@@ -133,13 +133,3 @@ impl Client for HttpClient {
Ok(responses)
}
}
fn parse_request_id(raw: Option<&JsonRawValue>) -> Result<u64, Error> {
match raw {
None => Err(Error::InvalidRequestId),
Some(id) => {
let id = serde_json::from_str(id.get()).map_err(Error::ParseError)?;
Ok(id)
}
}
}
use crate::v2::{
error::{JsonRpcErrorCode, JsonRpcErrorObjectAlloc},
error::{JsonRpcError, JsonRpcErrorCode, JsonRpcErrorObject},
params::JsonRpcParams,
};
use crate::{traits::Client, Error, HttpClientBuilder, JsonValue};
......@@ -107,9 +107,12 @@ async fn run_request_with_response(response: String) -> Result<JsonValue, Error>
client.request("say_hello", JsonRpcParams::NoParams).await
}
fn assert_jsonrpc_error_response(error: Error, code: JsonRpcErrorObjectAlloc) {
match &error {
Error::Request(e) => assert_eq!(e.error, code),
e => panic!("Expected error: \"{}\", got: {:?}", error, e),
fn assert_jsonrpc_error_response(err: Error, exp: JsonRpcErrorObject) {
match &err {
Error::Request(e) => {
let this: JsonRpcError = serde_json::from_str(&e).unwrap();
assert_eq!(this.error, exp);
}
e => panic!("Expected error: \"{}\", got: {:?}", err, e),
};
}
......@@ -34,8 +34,9 @@ use hyper::{
Error as HyperError,
};
use jsonrpsee_types::error::{CallError, Error, GenericTransportError};
use jsonrpsee_types::v2::request::{JsonRpcInvalidRequest, JsonRpcRequest};
use jsonrpsee_types::v2::{error::JsonRpcErrorCode, params::RpcParams};
use jsonrpsee_types::v2::error::JsonRpcErrorCode;
use jsonrpsee_types::v2::params::{Id, RpcParams};
use jsonrpsee_types::v2::request::{JsonRpcInvalidRequest, JsonRpcNotification, JsonRpcRequest};
use jsonrpsee_utils::hyper_helpers::read_response_to_body;
use jsonrpsee_utils::server::helpers::{collect_batch_response, send_error};
use jsonrpsee_utils::server::rpc_module::{MethodSink, RpcModule};
......@@ -162,7 +163,7 @@ impl Server {
if let Some(method) = methods.get(&*req.method) {
let params = RpcParams::new(req.params.map(|params| params.get()));
// NOTE(niklasad1): connection ID is unused thus hardcoded to `0`.
if let Err(err) = (method)(req.id, params, &tx, 0) {
if let Err(err) = (method)(req.id.clone(), params, &tx, 0) {
log::error!(
"execution of method call '{}' failed: {:?}, request id={:?}",
req.method,
......@@ -211,6 +212,8 @@ impl Server {
// Our [issue](https://github.com/paritytech/jsonrpsee/issues/296).
if let Ok(req) = serde_json::from_slice::<JsonRpcRequest>(&body) {
execute(&tx, req);
} else if let Ok(_req) = serde_json::from_slice::<JsonRpcNotification>(&body) {
return Ok::<_, HyperError>(response::ok_response("".into()));
} else if let Ok(batch) = serde_json::from_slice::<Vec<JsonRpcRequest>>(&body) {
if !batch.is_empty() {
single = false;
......@@ -218,8 +221,10 @@ impl Server {
execute(&tx, req);
}
} else {
send_error(None, &tx, JsonRpcErrorCode::InvalidRequest.into());
send_error(Id::Null, &tx, JsonRpcErrorCode::InvalidRequest.into());
}
} else if let Ok(_batch) = serde_json::from_slice::<Vec<JsonRpcNotification>>(&body) {
return Ok::<_, HyperError>(response::ok_response("".into()));
} else {
log::error!(
"[service_fn], Cannot parse request body={:?}",
......@@ -227,7 +232,7 @@ impl Server {
);
let (id, code) = match serde_json::from_slice::<JsonRpcInvalidRequest>(&body) {
Ok(req) => (req.id, JsonRpcErrorCode::InvalidRequest),
Err(_) => (None, JsonRpcErrorCode::ParseError),
Err(_) => (Id::Null, JsonRpcErrorCode::ParseError),
};
send_error(id, &tx, code.into());
}
......
......@@ -83,7 +83,7 @@ async fn invalid_single_method_call() {
let req = r#"{"jsonrpc":"2.0","method":1, "params": "bar"}"#;
let response = http_request(req.into(), uri.clone()).await.unwrap();
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, invalid_request(Id::Null));
assert_eq!(response.body, parse_error(Id::Null));
}
#[tokio::test]
......@@ -169,14 +169,11 @@ async fn batched_notifications() {
let addr = server().await;
let uri = to_http_uri(addr);
let req = r#"[
{"jsonrpc": "2.0", "method": "notif", "params": [1,2,4]},
{"jsonrpc": "2.0", "method": "notif", "params": [7]}
]"#;
let req = r#"[{"jsonrpc": "2.0", "method": "notif", "params": [1,2,4]},{"jsonrpc": "2.0", "method": "notif", "params": [7]}]"#;
let response = http_request(req.into(), uri).await.unwrap();
assert_eq!(response.status, StatusCode::OK);
// Note: this is *not* according to spec. Response should be the empty string, `""`.
assert_eq!(response.body, r#"[{"jsonrpc":"2.0","result":"","id":null},{"jsonrpc":"2.0","result":"","id":null}]"#);
// Note: on HTTP acknowledge the notification with an empty response.
assert_eq!(response.body, "");
}
#[tokio::test]
......@@ -248,3 +245,14 @@ async fn invalid_request_object() {
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, invalid_request(Id::Num(1)));
}
#[tokio::test]
async fn notif_works() {
let addr = server().await;
let uri = to_http_uri(addr);
let req = r#"{"jsonrpc":"2.0","method":"bar"}"#;
let response = http_request(req.into(), uri).await.unwrap();
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, "");
}
......@@ -11,7 +11,7 @@ documentation = "https://docs.rs/jsonrpsee-types"
[dependencies]
async-trait = "0.1"
beef = "0.5"
beef = { version = "0.5", features = ["impl_serde"] }
futures-channel = { version = "0.3", features = ["sink"] }
futures-util = { version = "0.3", default-features = false, features = ["std", "sink", "channel"] }
log = { version = "0.4", default-features = false }
......
use crate::v2::error::JsonRpcErrorAlloc;
use std::fmt;
/// Convenience type for displaying errors.
#[derive(Clone, Debug, PartialEq)]
pub struct Mismatch<T> {
......@@ -16,10 +14,6 @@ impl<T: fmt::Display> fmt::Display for Mismatch<T> {
}
}
/// Invalid params.
#[derive(Debug)]
pub struct InvalidParams;
/// Error that occurs when a call failed.
#[derive(Debug, thiserror::Error)]
pub enum CallError {
......@@ -31,12 +25,6 @@ pub enum CallError {
Failed(#[source] Box<dyn std::error::Error + Send + Sync>),
}
impl From<InvalidParams> for CallError {
fn from(_params: InvalidParams) -> Self {
Self::InvalidParams
}
}
/// Error type.
#[derive(Debug, thiserror::Error)]
pub enum Error {
......@@ -48,7 +36,7 @@ pub enum Error {
Transport(#[source] Box<dyn std::error::Error + Send + Sync>),
/// JSON-RPC request error.
#[error("JSON-RPC request error: {0:?}")]
Request(#[source] JsonRpcErrorAlloc),
Request(String),
/// Frontend/backend channel error.
#[error("Frontend/backend channel error: {0}")]
Internal(#[source] futures_channel::mpsc::SendError),
......
......@@ -2,57 +2,30 @@ use crate::v2::params::{Id, TwoPointZero};
use serde::de::Deserializer;
use serde::ser::Serializer;
use serde::{Deserialize, Serialize};
use serde_json::value::{RawValue, Value as JsonValue};
use serde_json::value::RawValue;
use std::fmt;
use thiserror::Error;
/// [Failed JSON-RPC response object](https://www.jsonrpc.org/specification#response_object).
#[derive(Serialize, Debug)]
#[derive(Serialize, Deserialize, Debug, PartialEq)]
pub struct JsonRpcError<'a> {
/// JSON-RPC version.
pub jsonrpc: TwoPointZero,
/// Error.
#[serde(borrow)]
pub error: JsonRpcErrorObject<'a>,
/// Request ID
pub id: Option<&'a RawValue>,
}
/// [Failed JSON-RPC response object with allocations](https://www.jsonrpc.org/specification#response_object).
#[derive(Error, Debug, Deserialize, PartialEq)]
pub struct JsonRpcErrorAlloc {
/// JSON-RPC version.
pub jsonrpc: TwoPointZero,
/// JSON-RPC error object.
pub error: JsonRpcErrorObjectAlloc,
/// Request ID.
pub id: Id,
pub id: Id<'a>,
}
impl fmt::Display for JsonRpcErrorAlloc {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{:?}: {:?}: {:?}", self.jsonrpc, self.error, self.id)
impl<'a> fmt::Display for JsonRpcError<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", serde_json::to_string(&self).expect("infallible; qed"))
}
}
/// JSON-RPC error object.
#[derive(Debug, PartialEq, Clone, Deserialize)]
#[serde(deny_unknown_fields)]
pub struct JsonRpcErrorObjectAlloc {
/// Code
pub code: JsonRpcErrorCode,
/// Message
pub message: String,
/// Optional data
pub data: Option<JsonValue>,
}
impl From<JsonRpcErrorCode> for JsonRpcErrorObjectAlloc {
fn from(code: JsonRpcErrorCode) -> Self {
Self { code, message: code.message().to_owned(), data: None }
}
}
/// JSON-RPC error object with no extra allocations.
#[derive(Debug, Serialize)]
#[derive(Debug, Deserialize, Serialize, Clone)]
#[serde(deny_unknown_fields)]
pub struct JsonRpcErrorObject<'a> {
/// Code
......@@ -61,6 +34,7 @@ pub struct JsonRpcErrorObject<'a> {
pub message: &'a str,
/// Optional data
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(borrow)]
pub data: Option<&'a RawValue>,
}
......@@ -70,6 +44,14 @@ impl<'a> From<JsonRpcErrorCode> for JsonRpcErrorObject<'a> {
}
}
impl<'a> PartialEq for JsonRpcErrorObject<'a> {
fn eq(&self, other: &Self) -> bool {
let this_raw = self.data.map(|r| r.get());
let other_raw = self.data.map(|r| r.get());
self.code == other.code && self.message == other.message && this_raw == other_raw
}
}
/// Parse error code.
pub const PARSE_ERROR_CODE: i32 = -32700;
/// Internal error code.
......@@ -180,47 +162,44 @@ impl serde::Serialize for JsonRpcErrorCode {
#[cfg(test)]
mod tests {
use super::{
Id, JsonRpcError, JsonRpcErrorAlloc, JsonRpcErrorCode, JsonRpcErrorObject, JsonRpcErrorObjectAlloc,
TwoPointZero,
};
use super::{Id, JsonRpcError, JsonRpcErrorCode, JsonRpcErrorObject, TwoPointZero};
#[test]
fn deserialize_works() {
let ser = r#"{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error"},"id":null}"#;
let err: JsonRpcErrorAlloc = serde_json::from_str(ser).unwrap();
assert_eq!(err.jsonrpc, TwoPointZero);
assert_eq!(
err.error,
JsonRpcErrorObjectAlloc { code: JsonRpcErrorCode::ParseError, message: "Parse error".into(), data: None }
);
assert_eq!(err.id, Id::Null);
let exp = JsonRpcError {
jsonrpc: TwoPointZero,
error: JsonRpcErrorObject { code: JsonRpcErrorCode::ParseError, message: "Parse error".into(), data: None },
id: Id::Null,
};
let err: JsonRpcError = serde_json::from_str(ser).unwrap();
assert_eq!(exp, err);
}
#[test]
fn deserialize_with_optional_data() {
let ser = r#"{"jsonrpc":"2.0","error":{"code":-32700,"message":"Parse error", "data":"vegan"},"id":null}"#;
let err: JsonRpcErrorAlloc = serde_json::from_str(ser).unwrap();
assert_eq!(err.jsonrpc, TwoPointZero);
assert_eq!(
err.error,
JsonRpcErrorObjectAlloc {
let data = serde_json::value::to_raw_value(&"vegan").unwrap();
let exp = JsonRpcError {
jsonrpc: TwoPointZero,
error: JsonRpcErrorObject {
code: JsonRpcErrorCode::ParseError,
message: "Parse error".into(),
data: Some("vegan".into())
}
);
assert_eq!(err.id, Id::Null);
data: Some(&*data),
},
id: Id::Null,
};
let err: JsonRpcError = serde_json::from_str(ser).unwrap();
assert_eq!(exp, err);
}
#[test]
fn serialize_works() {
let exp = r#"{"jsonrpc":"2.0","error":{"code":-32603,"message":"Internal error"},"id":1337}"#;
let raw_id = serde_json::value::to_raw_value(&1337).unwrap();
let err = JsonRpcError {
jsonrpc: TwoPointZero,
error: JsonRpcErrorObject { code: JsonRpcErrorCode::InternalError, message: "Internal error", data: None },
id: Some(&*raw_id),
id: Id::Number(1337),
};
let ser = serde_json::to_string(&err).unwrap();
assert_eq!(exp, ser);
......
use crate::error::Error;
use serde::de::DeserializeOwned;
use serde_json::value::RawValue;
/// JSON-RPC error related types.
pub mod error;
/// JSON_RPC params related types.
......@@ -10,14 +6,3 @@ pub mod params;
pub mod request;
/// JSON-RPC response object related types.
pub mod response;
/// Parse request ID from RawValue.
pub fn parse_request_id<T: DeserializeOwned>(raw: Option<&RawValue>) -> Result<T, Error> {
match raw {
None => Err(Error::InvalidRequestId),
Some(v) => {
let val = serde_json::from_str(v.get()).map_err(|_| Error::InvalidRequestId)?;
Ok(val)
}
}
}
use crate::error::InvalidParams;
use crate::error::CallError;
use alloc::collections::BTreeMap;
use beef::Cow;
use serde::de::{self, Deserializer, Unexpected, Visitor};
use serde::ser::Serializer;
use serde::{Deserialize, Serialize};
......@@ -26,7 +27,7 @@ pub struct JsonRpcNotificationParamsAlloc<T> {
}
/// JSON-RPC v2 marker type.
#[derive(Debug, Default, PartialEq)]
#[derive(Clone, Copy, Debug, Default, PartialEq)]
pub struct TwoPointZero;
struct TwoPointZeroVisitor;
......@@ -78,18 +79,18 @@ impl<'a> RpcParams<'a> {
}
/// Attempt to parse all parameters as array or map into type T
pub fn parse<T>(self) -> Result<T, InvalidParams>
pub fn parse<T>(self) -> Result<T, CallError>
where
T: Deserialize<'a>,
{
match self.0 {
None => Err(InvalidParams),
Some(params) => serde_json::from_str(params).map_err(|_| InvalidParams),
None => Err(CallError::InvalidParams),
Some(params) => serde_json::from_str(params).map_err(|_| CallError::InvalidParams),
}
}
/// Attempt to parse only the first parameter from an array into type T
pub fn one<T>(self) -> Result<T, InvalidParams>
pub fn one<T>(self) -> Result<T, CallError>
where
T: Deserialize<'a>,
{
......@@ -157,16 +158,17 @@ impl From<SubscriptionId> for JsonValue {
#[derive(Debug, PartialEq, Clone, Hash, Eq, Deserialize, Serialize)]
#[serde(deny_unknown_fields)]
#[serde(untagged)]
pub enum Id {
pub enum Id<'a> {
/// Null
Null,
/// Numeric id
Number(u64),
/// String id
Str(String),
#[serde(borrow)]
Str(Cow<'a, str>),
}
impl Id {
impl<'a> Id<'a> {
/// If the Id is a number, returns the associated number. Returns None otherwise.
pub fn as_number(&self) -> Option<&u64> {
match self {
......@@ -192,6 +194,58 @@ impl Id {
}
}
/// Untyped JSON-RPC ID.
// TODO(niklasad1): this should be enforced to only accept: String, Number, or Null.
pub type JsonRpcRawId<'a> = Option<&'a serde_json::value::RawValue>;
#[cfg(test)]
mod test {
use super::{Cow, Id, JsonValue, RpcParams};
#[test]
fn id_deserialization() {
let s = r#""2""#;
let deserialized: Id = serde_json::from_str(s).unwrap();
assert_eq!(deserialized, Id::Str("2".into()));
let s = r#"2"#;
let deserialized: Id = serde_json::from_str(s).unwrap();
assert_eq!(deserialized, Id::Number(2));
let s = r#""2x""#;
let deserialized: Id = serde_json::from_str(s).unwrap();
assert_eq!(deserialized, Id::Str(Cow::const_str("2x")));
let s = r#"[1337]"#;
assert!(serde_json::from_str::<Id>(s).is_err());
let s = r#"[null, 0, 2, "3"]"#;
let deserialized: Vec<Id> = serde_json::from_str(s).unwrap();
assert_eq!(deserialized, vec![Id::Null, Id::Number(0), Id::Number(2), Id::Str("3".into())]);
}
#[test]
fn id_serialization() {
let d =
vec![Id::Null, Id::Number(0), Id::Number(2), Id::Number(3), Id::Str("3".into()), Id::Str("test".into())];
let serialized = serde_json::to_string(&d).unwrap();
assert_eq!(serialized, r#"[null,0,2,3,"3","test"]"#);
}
#[test]
fn params_parse() {
let none = RpcParams::new(None);
assert!(none.one::<u64>().is_err());
let array_params = RpcParams::new(Some("[1, 2, 3]"));
let arr: Result<[u64; 3], _> = array_params.parse();
assert!(arr.is_ok());
let arr: Result<(u64, u64, u64), _> = array_params.parse();
assert!(arr.is_ok());
let array_one = RpcParams::new(Some("[1]"));
let one: Result<u64, _> = array_one.one();
assert!(one.is_ok());
let object_params = RpcParams::new(Some(r#"{"beef":99,"dinner":0}"#));
let obj: Result<JsonValue, _> = object_params.parse();
assert!(obj.is_ok());
}
}
use crate::v2::params::{Id, JsonRpcNotificationParams, JsonRpcParams, TwoPointZero};
use crate::v2::params::{Id, JsonRpcParams, TwoPointZero};
use beef::Cow;
use serde::{Deserialize, Serialize};
use serde_json::value::RawValue;
......@@ -11,7 +11,7 @@ pub struct JsonRpcRequest<'a> {
pub jsonrpc: TwoPointZero,
/// Request ID
#[serde(borrow)]
pub id: Option<&'a RawValue>,
pub id: Id<'a>,
/// Name of the method to be invoked.
#[serde(borrow)]
pub method: Cow<'a, str>,
......@@ -21,22 +21,24 @@ pub struct JsonRpcRequest<'a> {
}
/// Invalid request with known request ID.
#[derive(Deserialize, Debug)]
#[derive(Deserialize, Debug, PartialEq)]
pub struct JsonRpcInvalidRequest<'a> {
/// Request ID
#[serde(borrow)]
pub id: Option<&'a RawValue>,
pub id: Id<'a>,
}
/// JSON-RPC notification (a request object without a request ID).
#[derive(Serialize, Deserialize, Debug)]
#[serde(deny_unknown_fields)]
pub struct JsonRpcNotification<'a> {
/// JSON-RPC version.
pub jsonrpc: TwoPointZero,
/// Name of the method to be invoked.
pub method: &'a str,
/// Parameter values of the request.
pub params: JsonRpcNotificationParams<'a>,
#[serde(borrow)]
pub params: Option<&'a RawValue>,
}
/// Serializable [JSON-RPC object](https://www.jsonrpc.org/specification#request-object)
......@@ -47,14 +49,14 @@ pub struct JsonRpcCallSer<'a> {
/// Name of the method to be invoked.
pub method: &'a str,
/// Request ID
pub id: Id,
pub id: Id<'a>,
/// Parameter values of the request.
pub params: JsonRpcParams<'a>,
}
impl<'a> JsonRpcCallSer<'a> {
/// Create a new serializable JSON-RPC request.
pub fn new(id: Id, method: &'a str, params: JsonRpcParams<'a>) -> Self {
pub fn new(id: Id<'a>, method: &'a str, params: JsonRpcParams<'a>) -> Self {
Self { jsonrpc: TwoPointZero, id, method, params }
}
}
......@@ -76,3 +78,72 @@ impl<'a> JsonRpcNotificationSer<'a> {
Self { jsonrpc: TwoPointZero, method, params }
}
}
#[cfg(test)]
mod test {
use super::{
Id, JsonRpcCallSer, JsonRpcInvalidRequest, JsonRpcNotification, JsonRpcNotificationSer, JsonRpcRequest,
TwoPointZero,
};
#[test]
fn deserialize_valid_call_works() {
let ser = r#"{"jsonrpc":"2.0","method":"say_hello","params":[1,"bar"],"id":1}"#;
let dsr: JsonRpcRequest = serde_json::from_str(ser).unwrap();
assert_eq!(dsr.method, "say_hello");