[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

Author: niklasad1

http-client/src/client.rs

@@ -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)
-		}
-	}
-}

http-client/src/tests.rs

@@ -1,5 +1,5 @@
 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),
 	};
 }

http-server/src/server.rs

@@ -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,23 +212,27 @@ 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;
 								for req in batch {
 									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={:?}",
 								String::from_utf8_lossy(&body[..cmp::min(body.len(), 1024)])
 							);
 							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());
 						}

http-server/src/tests.rs

@@ -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, "");
+}

types/Cargo.toml

@@ -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 }

types/src/error.rs

@@ -1,6 +1,4 @@
-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),

types/src/v2/error.rs

@@ -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);