From 9d785c84d588e8e121c67824f51ce3b2cb77d959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Sel=C3=A1f?= Date: Sat, 2 Oct 2021 11:33:24 +0200 Subject: [PATCH 1/6] omit params from requests instead of sending null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Szabolcs Seláf --- core/src/types/params.rs | 6 ++++++ core/src/types/request.rs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/types/params.rs b/core/src/types/params.rs index 2eac38606..d60b614a4 100644 --- a/core/src/types/params.rs +++ b/core/src/types/params.rs @@ -36,6 +36,12 @@ impl Params { p => Err(Error::invalid_params_with_details("No parameters were expected", p)), } } + + /// Check for None + pub fn is_none(&self) -> bool { + log::debug!("is_none called on {:?}", self); + matches!(*self, Params::None) + } } impl From for Value { diff --git a/core/src/types/request.rs b/core/src/types/request.rs index adbf0eccd..1a0e7c295 100644 --- a/core/src/types/request.rs +++ b/core/src/types/request.rs @@ -12,7 +12,7 @@ pub struct MethodCall { pub method: String, /// A Structured value that holds the parameter values to be used /// during the invocation of the method. This member MAY be omitted. - #[serde(default = "default_params")] + #[serde(default = "default_params", skip_serializing_if = "Params::is_none")] pub params: Params, /// An identifier established by the Client that MUST contain a String, /// Number, or NULL value if included. If it is not included it is assumed From 7bf2b7b35f8dff14c8a8ff2d77f3bf58846b3acd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Sel=C3=A1f?= Date: Sat, 2 Oct 2021 11:39:21 +0200 Subject: [PATCH 2/6] remove debug message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Szabolcs Seláf --- core/src/types/params.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/types/params.rs b/core/src/types/params.rs index d60b614a4..525624bd9 100644 --- a/core/src/types/params.rs +++ b/core/src/types/params.rs @@ -39,7 +39,6 @@ impl Params { /// Check for None pub fn is_none(&self) -> bool { - log::debug!("is_none called on {:?}", self); matches!(*self, Params::None) } } From 6854e8ef0a3ec311129f044251849ef9d3cbcbaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Sel=C3=A1f?= Date: Sat, 2 Oct 2021 12:02:56 +0200 Subject: [PATCH 3/6] add test for omitting params from requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Szabolcs Seláf --- core/src/types/params.rs | 8 ++++++++ core/src/types/request.rs | 15 +++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/core/src/types/params.rs b/core/src/types/params.rs index 525624bd9..cdf0f8c42 100644 --- a/core/src/types/params.rs +++ b/core/src/types/params.rs @@ -115,4 +115,12 @@ mod tests { let params: (u64,) = Params::Array(vec![Value::from(1)]).parse().unwrap(); assert_eq!(params, (1,)); } + + #[test] + fn detect_none() { + let none = Params::None; + assert!(none.is_none()); + let some = Params::Array(vec![]); + assert!(!some.is_none()); + } } diff --git a/core/src/types/request.rs b/core/src/types/request.rs index 1a0e7c295..8c9e99f95 100644 --- a/core/src/types/request.rs +++ b/core/src/types/request.rs @@ -105,6 +105,21 @@ mod tests { ); } + #[test] + fn call_serizalize_without_params() { + use serde_json; + + let m = MethodCall { + jsonrpc: Some(Version::V2), + method: "status".to_owned(), + params: Params::None, + id: Id::Num(1), + }; + + let serialized = serde_json::to_string(&m).unwrap(); + assert_eq!(serialized, r#"{"jsonrpc":"2.0","method":"status","id":1}"#); + } + #[test] fn notification_serialize() { use serde_json; From 60aeb01597ea45ba46b3bb180dc00dc65851b2ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Sel=C3=A1f?= Date: Tue, 5 Oct 2021 21:41:58 +0200 Subject: [PATCH 4/6] fix typo in test case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Tomasz Drwięga --- core/src/types/request.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/types/request.rs b/core/src/types/request.rs index 8c9e99f95..d53714984 100644 --- a/core/src/types/request.rs +++ b/core/src/types/request.rs @@ -106,7 +106,7 @@ mod tests { } #[test] - fn call_serizalize_without_params() { + fn call_serialize_without_params() { use serde_json; let m = MethodCall { From ececfbfb0bcd5294a1a9f3a22a683ba7790f04b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Sel=C3=A1f?= Date: Wed, 6 Oct 2021 11:58:44 +0200 Subject: [PATCH 5/6] add test for optional params MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Szabolcs Seláf --- derive/tests/macros.rs | 50 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/derive/tests/macros.rs b/derive/tests/macros.rs index 1f4483672..34f8470c0 100644 --- a/derive/tests/macros.rs +++ b/derive/tests/macros.rs @@ -30,6 +30,10 @@ pub trait Rpc { #[rpc(name = "raw", params = "raw")] fn raw(&self, params: Params) -> Result; + /// Squares a number, default value is 1. + #[rpc(name = "sqr")] + fn sqr(&self, a: Option) -> Result; + /// Handles a notification. #[rpc(name = "notify")] fn notify(&self, a: u64); @@ -55,6 +59,11 @@ impl Rpc for RpcImpl { Ok("OK".into()) } + fn sqr(&self, a: Option) -> Result { + let a = a.unwrap_or(1); + Ok(a * a) + } + fn notify(&self, a: u64) { println!("Received `notify` with value: {}", a); } @@ -222,6 +231,47 @@ fn should_accept_any_raw_params() { assert_eq!(expected, result4); } +#[test] +fn should_accept_optional_param() { + let mut io = IoHandler::new(); + let rpc = RpcImpl::default(); + io.extend_with(rpc.to_delegate()); + + // when + let req1 = r#"{"jsonrpc":"2.0","id":1,"method":"sqr","params":[2]}"#; + let req2 = r#"{"jsonrpc":"2.0","id":1,"method":"sqr"}"#; + + let res1 = io.handle_request_sync(req1); + let res2 = io.handle_request_sync(req2); + + // then + let result1: Response = serde_json::from_str(&res1.unwrap()).unwrap(); + assert_eq!( + result1, + serde_json::from_str( + r#"{ + "jsonrpc": "2.0", + "result": 4, + "id": 1 + }"# + ) + .unwrap() + ); + + let result2: Response = serde_json::from_str(&res2.unwrap()).unwrap(); + assert_eq!( + result2, + serde_json::from_str( + r#"{ + "jsonrpc": "2.0", + "result": 1, + "id": 1 + }"# + ) + .unwrap() + ); +} + #[test] fn should_accept_only_notifications() { let mut io = IoHandler::new(); From c89ced2b0cd249b48f661f11a40168e7b731e540 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szabolcs=20Sel=C3=A1f?= Date: Wed, 6 Oct 2021 14:51:12 +0200 Subject: [PATCH 6/6] additional empty params variations for optinal testing --- derive/tests/macros.rs | 46 ++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/derive/tests/macros.rs b/derive/tests/macros.rs index 34f8470c0..4ab1d5058 100644 --- a/derive/tests/macros.rs +++ b/derive/tests/macros.rs @@ -239,37 +239,39 @@ fn should_accept_optional_param() { // when let req1 = r#"{"jsonrpc":"2.0","id":1,"method":"sqr","params":[2]}"#; - let req2 = r#"{"jsonrpc":"2.0","id":1,"method":"sqr"}"#; + let req2 = r#"{"jsonrpc":"2.0","id":1,"method":"sqr","params":[]}"#; + let req3 = r#"{"jsonrpc":"2.0","id":1,"method":"sqr","params":null}"#; + let req4 = r#"{"jsonrpc":"2.0","id":1,"method":"sqr"}"#; let res1 = io.handle_request_sync(req1); let res2 = io.handle_request_sync(req2); - - // then - let result1: Response = serde_json::from_str(&res1.unwrap()).unwrap(); - assert_eq!( - result1, - serde_json::from_str( - r#"{ + let res3 = io.handle_request_sync(req3); + let res4 = io.handle_request_sync(req4); + let expected1 = r#"{ "jsonrpc": "2.0", "result": 4, "id": 1 - }"# - ) - .unwrap() - ); - - let result2: Response = serde_json::from_str(&res2.unwrap()).unwrap(); - assert_eq!( - result2, - serde_json::from_str( - r#"{ + }"#; + let expected1: Response = serde_json::from_str(expected1).unwrap(); + let expected2 = r#"{ "jsonrpc": "2.0", "result": 1, "id": 1 - }"# - ) - .unwrap() - ); + }"#; + let expected2: Response = serde_json::from_str(expected2).unwrap(); + + // then + let result1: Response = serde_json::from_str(&res1.unwrap()).unwrap(); + assert_eq!(expected1, result1); + + let result2: Response = serde_json::from_str(&res2.unwrap()).unwrap(); + assert_eq!(expected2, result2); + + let result3: Response = serde_json::from_str(&res3.unwrap()).unwrap(); + assert_eq!(expected2, result3); + + let result4: Response = serde_json::from_str(&res4.unwrap()).unwrap(); + assert_eq!(expected2, result4); } #[test]