Skip to content

Commit

Permalink
feat(types): serialize id in Response before result/error fie…
Browse files Browse the repository at this point in the history
…lds (#1421)
  • Loading branch information
ArtificialPB authored Jul 4, 2024
1 parent 6bc58fe commit 87999cf
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 49 deletions.
2 changes: 1 addition & 1 deletion core/src/server/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ mod tests {
let rp = &Response::new(result, Id::Number(1));

assert!(serde_json::to_writer(&mut writer, rp).is_ok());
assert_eq!(String::from_utf8(writer.into_bytes()).unwrap(), r#"{"jsonrpc":"2.0","result":"success","id":1}"#);
assert_eq!(String::from_utf8(writer.into_bytes()).unwrap(), r#"{"jsonrpc":"2.0","id":1,"result":"success"}"#);
}

#[test]
Expand Down
8 changes: 4 additions & 4 deletions core/src/server/method_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ mod tests {
builder.append(&method).unwrap();
let batch = builder.finish();

assert_eq!(batch.0, r#"[{"jsonrpc":"2.0","result":"a","id":1}]"#)
assert_eq!(batch.0, r#"[{"jsonrpc":"2.0","id":1,"result":"a"}]"#)
}

#[test]
Expand All @@ -492,14 +492,14 @@ mod tests {
builder.append(&m1).unwrap();
let batch = builder.finish();

assert_eq!(batch.0, r#"[{"jsonrpc":"2.0","result":"a","id":1},{"jsonrpc":"2.0","result":"a","id":1}]"#)
assert_eq!(batch.0, r#"[{"jsonrpc":"2.0","id":1,"result":"a"},{"jsonrpc":"2.0","id":1,"result":"a"}]"#)
}

#[test]
fn batch_empty_err() {
let batch = BatchResponseBuilder::new_with_limit(1024).finish();

let exp_err = r#"{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"},"id":null}"#;
let exp_err = r#"{"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"Invalid request"}}"#;
assert_eq!(batch.0, exp_err);
}

Expand All @@ -510,7 +510,7 @@ mod tests {

let batch = BatchResponseBuilder::new_with_limit(63).append(&method).unwrap_err();

let exp_err = r#"{"jsonrpc":"2.0","error":{"code":-32011,"message":"The batch response was too large","data":"Exceeded max limit of 63"},"id":null}"#;
let exp_err = r#"{"jsonrpc":"2.0","id":null,"error":{"code":-32011,"message":"The batch response was too large","data":"Exceeded max limit of 63"}}"#;
assert_eq!(batch.result, exp_err);
}
}
14 changes: 7 additions & 7 deletions server/src/tests/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ async fn single_method_call_with_multiple_params_of_different_types() {
async fn single_method_call_with_faulty_params_returns_err() {
let (addr, _handle) = server().with_default_timeout().await.unwrap();
let uri = to_http_uri(addr);
let expected = r#"{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params","data":"invalid type: string \"this should be a number\", expected u64 at line 1 column 26"},"id":1}"#;
let expected = r#"{"jsonrpc":"2.0","id":1,"error":{"code":-32602,"message":"Invalid params","data":"invalid type: string \"this should be a number\", expected u64 at line 1 column 26"}}"#;

let req = r#"{"jsonrpc":"2.0","method":"add", "params":["this should be a number"],"id":1}"#;
let response = http_request(req.into(), uri).with_default_timeout().await.unwrap().unwrap();
Expand Down Expand Up @@ -212,7 +212,7 @@ async fn valid_batched_method_calls() {
assert_eq!(response.status, StatusCode::OK);
assert_eq!(
response.body,
r#"[{"jsonrpc":"2.0","result":3,"id":1},{"jsonrpc":"2.0","result":7,"id":2},{"jsonrpc":"2.0","result":"lo","id":3},{"jsonrpc":"2.0","result":11,"id":4}]"#
r#"[{"jsonrpc":"2.0","id":1,"result":3},{"jsonrpc":"2.0","id":2,"result":7},{"jsonrpc":"2.0","id":3,"result":"lo"},{"jsonrpc":"2.0","id":4,"result":11}]"#
);
}

Expand Down Expand Up @@ -278,7 +278,7 @@ async fn batch_with_mixed_calls() {
{"foo": "boo"},
{"jsonrpc": "2.0", "method": "foo.get", "params": {"name": "myself"}, "id": "5"}
]"#;
let res = r#"[{"jsonrpc":"2.0","result":7,"id":"1"},{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"},"id":null},{"jsonrpc":"2.0","error":{"code":-32601,"message":"Method not found"},"id":"5"}]"#;
let res = r#"[{"jsonrpc":"2.0","id":"1","result":7},{"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"Invalid request"}},{"jsonrpc":"2.0","id":"5","error":{"code":-32601,"message":"Method not found"}}]"#;
let response = http_request(req.into(), uri.clone()).with_default_timeout().await.unwrap().unwrap();
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, res);
Expand All @@ -295,7 +295,7 @@ async fn batch_notif_without_params_works() {
{"jsonrpc": "2.0", "method": "add", "params": [1,2,4], "id": "1"},
{"jsonrpc": "2.0", "method": "add"}
]"#;
let res = r#"[{"jsonrpc":"2.0","result":7,"id":"1"}]"#;
let res = r#"[{"jsonrpc":"2.0","id":"1","result":7}]"#;
let response = http_request(req.into(), uri.clone()).with_default_timeout().await.unwrap().unwrap();
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, res);
Expand Down Expand Up @@ -346,20 +346,20 @@ async fn whitespace_is_not_significant() {

let req = r#" {"jsonrpc":"2.0","method":"add", "params":[1, 2],"id":1}"#;
let response = http_request(req.into(), uri.clone()).await.unwrap();
let expected = r#"{"jsonrpc":"2.0","result":3,"id":1}"#;
let expected = r#"{"jsonrpc":"2.0","id":1,"result":3}"#;
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, expected);

let req = r#" [{"jsonrpc":"2.0","method":"add", "params":[1, 2],"id":1}]"#;
let response = http_request(req.into(), uri.clone()).await.unwrap();
let expected = r#"[{"jsonrpc":"2.0","result":3,"id":1}]"#;
let expected = r#"[{"jsonrpc":"2.0","id":1,"result":3}]"#;
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, expected);

// Up to 127 whitespace chars are accepted.
let req = format!("{}{}", " ".repeat(127), r#"{"jsonrpc":"2.0","method":"add", "params":[1, 2],"id":1}"#);
let response = http_request(req.into(), uri.clone()).await.unwrap();
let expected = r#"{"jsonrpc":"2.0","result":3,"id":1}"#;
let expected = r#"{"jsonrpc":"2.0","id":1,"result":3}"#;
assert_eq!(response.status, StatusCode::OK);
assert_eq!(response.body, expected);

Expand Down
18 changes: 9 additions & 9 deletions server/src/tests/ws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ async fn batch_method_call_works() {
let response = client.send_request_text(batch).with_default_timeout().await.unwrap().unwrap();
assert_eq!(
response,
r#"[{"jsonrpc":"2.0","result":"Yawn!","id":123},{"jsonrpc":"2.0","result":"hello","id":1},{"jsonrpc":"2.0","result":"hello","id":2},{"jsonrpc":"2.0","result":"hello","id":3}]"#
r#"[{"jsonrpc":"2.0","id":123,"result":"Yawn!"},{"jsonrpc":"2.0","id":1,"result":"hello"},{"jsonrpc":"2.0","id":2,"result":"hello"},{"jsonrpc":"2.0","id":3,"result":"hello"}]"#
);
}

Expand All @@ -223,7 +223,7 @@ async fn batch_method_call_where_some_calls_fail() {

assert_eq!(
response,
r#"[{"jsonrpc":"2.0","result":"hello","id":1},{"jsonrpc":"2.0","error":{"code":-32000,"message":"MyAppError"},"id":2},{"jsonrpc":"2.0","result":79,"id":3}]"#
r#"[{"jsonrpc":"2.0","id":1,"result":"hello"},{"jsonrpc":"2.0","id":2,"error":{"code":-32000,"message":"MyAppError"}},{"jsonrpc":"2.0","id":3,"result":79}]"#
);
}

Expand Down Expand Up @@ -278,7 +278,7 @@ async fn whitespace_is_not_significant() {

let req = r#" [{"jsonrpc":"2.0","method":"add", "params":[1, 2],"id":1}]"#;
let response = client.send_request_text(req).await.unwrap();
assert_eq!(response, r#"[{"jsonrpc":"2.0","result":3,"id":1}]"#);
assert_eq!(response, r#"[{"jsonrpc":"2.0","id":1,"result":3}]"#);

// Up to 127 whitespace chars are accepted.
let req = format!("{}{}", " ".repeat(127), r#"{"jsonrpc":"2.0","method":"add", "params":[1, 2],"id":1}"#);
Expand All @@ -305,7 +305,7 @@ async fn single_method_call_with_params_works() {
async fn single_method_call_with_faulty_params_returns_err() {
let addr = server().await;
let mut client = WebSocketTestClient::new(addr).with_default_timeout().await.unwrap().unwrap();
let expected = r#"{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params","data":"invalid type: string \"should be a number\", expected u64 at line 1 column 21"},"id":1}"#;
let expected = r#"{"jsonrpc":"2.0","id":1,"error":{"code":-32602,"message":"Invalid params","data":"invalid type: string \"should be a number\", expected u64 at line 1 column 21"}}"#;

let req = r#"{"jsonrpc":"2.0","method":"add", "params":["should be a number"],"id":1}"#;
let response = client.send_request_text(req).with_default_timeout().await.unwrap().unwrap();
Expand Down Expand Up @@ -486,7 +486,7 @@ async fn valid_request_that_fails_to_execute_should_not_close_connection() {
// Good request, but causes error.
let req = r#"{"jsonrpc":"2.0","method":"call_fail","params":[],"id":123}"#;
let response = client.send_request_text(req).with_default_timeout().await.unwrap().unwrap();
assert_eq!(response, r#"{"jsonrpc":"2.0","error":{"code":-32000,"message":"MyAppError"},"id":123}"#);
assert_eq!(response, r#"{"jsonrpc":"2.0","id":123,"error":{"code":-32000,"message":"MyAppError"}}"#);

// Connection is still good.
let request = r#"{"jsonrpc":"2.0","method":"say_hello","id":333}"#;
Expand Down Expand Up @@ -585,9 +585,9 @@ async fn custom_subscription_id_works() {
let mut client = WebSocketTestClient::new(addr).with_default_timeout().await.unwrap().unwrap();

let sub = client.send_request_text(call("subscribe_hello", Vec::<()>::new(), Id::Num(0))).await.unwrap();
assert_eq!(&sub, r#"{"jsonrpc":"2.0","result":"0xdeadbeef","id":0}"#);
assert_eq!(&sub, r#"{"jsonrpc":"2.0","id":0,"result":"0xdeadbeef"}"#);
let unsub = client.send_request_text(call("unsubscribe_hello", vec!["0xdeadbeef"], Id::Num(1))).await.unwrap();
assert_eq!(&unsub, r#"{"jsonrpc":"2.0","result":true,"id":1}"#);
assert_eq!(&unsub, r#"{"jsonrpc":"2.0","id":1,"result":true}"#);
}

#[tokio::test]
Expand Down Expand Up @@ -694,7 +694,7 @@ async fn batch_with_mixed_calls() {
{"foo": "boo"},
{"jsonrpc": "2.0", "method": "foo.get", "params": {"name": "myself"}, "id": "5"}
]"#;
let res = r#"[{"jsonrpc":"2.0","result":7,"id":"1"},{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid request"},"id":null},{"jsonrpc":"2.0","error":{"code":-32601,"message":"Method not found"},"id":"5"}]"#;
let res = r#"[{"jsonrpc":"2.0","id":"1","result":7},{"jsonrpc":"2.0","id":null,"error":{"code":-32600,"message":"Invalid request"}},{"jsonrpc":"2.0","id":"5","error":{"code":-32601,"message":"Method not found"}}]"#;
let response = client.send_request_text(req.to_string()).with_default_timeout().await.unwrap().unwrap();
assert_eq!(response, res);
}
Expand All @@ -710,7 +710,7 @@ async fn batch_notif_without_params_works() {
{"jsonrpc": "2.0", "method": "add", "params": [1,2,4], "id": "1"},
{"jsonrpc": "2.0", "method": "add"}
]"#;
let res = r#"[{"jsonrpc":"2.0","result":7,"id":"1"}]"#;
let res = r#"[{"jsonrpc":"2.0","id":"1","result":7}]"#;
let response = client.send_request_text(req.to_string()).with_default_timeout().await.unwrap().unwrap();
assert_eq!(response, res);
}
Expand Down
34 changes: 17 additions & 17 deletions test-utils/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,56 +60,56 @@ pub fn to_http_uri(sockaddr: SocketAddr) -> Uri {
}

pub fn ok_response(result: Value, id: Id) -> String {
format!(r#"{{"jsonrpc":"2.0","result":{},"id":{}}}"#, result, serde_json::to_string(&id).unwrap())
format!(r#"{{"jsonrpc":"2.0","id":{},"result":{}}}"#, serde_json::to_string(&id).unwrap(), result)
}

pub fn method_not_found(id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32601,"message":"Method not found"}},"id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32601,"message":"Method not found"}}}}"#,
serde_json::to_string(&id).unwrap()
)
}

pub fn parse_error(id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32700,"message":"Parse error"}},"id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32700,"message":"Parse error"}}}}"#,
serde_json::to_string(&id).unwrap()
)
}

pub fn oversized_request(max_limit: u32) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32007,"message":"Request is too big","data":"Exceeded max limit of {max_limit}"}},"id":null}}"#
r#"{{"jsonrpc":"2.0","id":null,"error":{{"code":-32007,"message":"Request is too big","data":"Exceeded max limit of {max_limit}"}}}}"#
)
}

pub fn batches_not_supported() -> String {
r#"{"jsonrpc":"2.0","error":{"code":-32005,"message":"Batched requests are not supported by this server"},"id":null}"#.into()
r#"{"jsonrpc":"2.0","id":null,"error":{"code":-32005,"message":"Batched requests are not supported by this server"}}"#.into()
}

pub fn batches_too_large(max_limit: usize) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32010,"message":"The batch request was too large","data":"Exceeded max limit of {max_limit}"}},"id":null}}"#
r#"{{"jsonrpc":"2.0","id":null,"error":{{"code":-32010,"message":"The batch request was too large","data":"Exceeded max limit of {max_limit}"}}}}"#
)
}

pub fn batch_response_too_large(max_limit: usize) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32011,"message":"The batch response was too large","data":"Exceeded max limit of {max_limit}"}},"id":null}}"#
r#"{{"jsonrpc":"2.0","id":null,"error":{{"code":-32011,"message":"The batch response was too large","data":"Exceeded max limit of {max_limit}"}}}}"#
)
}

pub fn oversized_response(id: Id, max_limit: u32) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32008,"message":"Response is too big","data":"Exceeded max limit of {}"}},"id":{}}}"#,
max_limit,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32008,"message":"Response is too big","data":"Exceeded max limit of {}"}}}}"#,
serde_json::to_string(&id).unwrap(),
max_limit,
)
}

pub fn invalid_request(id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32600,"message":"Invalid request"}},"id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32600,"message":"Invalid request"}}}}"#,
serde_json::to_string(&id).unwrap()
)
}
Expand All @@ -121,7 +121,7 @@ pub fn invalid_batch(ids: Vec<Id>) -> String {
for (i, id) in ids.iter().enumerate() {
write!(
result,
r#"{{"jsonrpc":"2.0","error":{{"code":-32600,"message":"Invalid request"}},"id":{}}}{}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32600,"message":"Invalid request"}}}}{}"#,
serde_json::to_string(&id).unwrap(),
if i + 1 == ids.len() { "" } else { "," }
)
Expand All @@ -133,7 +133,7 @@ pub fn invalid_batch(ids: Vec<Id>) -> String {

pub fn invalid_params(id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32602,"message":"Invalid params"}},"id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32602,"message":"Invalid params"}}}}"#,
serde_json::to_string(&id).unwrap()
)
}
Expand All @@ -149,22 +149,22 @@ pub fn call<T: Serialize>(method: &str, params: Vec<T>, id: Id) -> String {

pub fn call_execution_failed(msg: &str, id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32000,"message":"{}"}},"id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32000,"message":"{}"}}}}"#,
serde_json::to_string(&id).unwrap(),
msg,
serde_json::to_string(&id).unwrap()
)
}

pub fn internal_error(id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32603,"message":"Internal error"}},"id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32603,"message":"Internal error"}}}}"#,
serde_json::to_string(&id).unwrap()
)
}

pub fn server_error(id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","error":{{"code":-32000,"message":"Server error"}},"id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32000,"message":"Server error"}}}}"#,
serde_json::to_string(&id).unwrap()
)
}
Expand All @@ -174,7 +174,7 @@ pub fn server_error(id: Id) -> String {
/// NOTE: works only for one subscription because the subscription ID is hardcoded.
pub fn server_subscription_id_response(id: Id) -> String {
format!(
r#"{{"jsonrpc":"2.0","result":"D3wwzU6vvoUUYehv4qoFzq42DZnLoAETeFzeyk8swH4o","id":{}}}"#,
r#"{{"jsonrpc":"2.0","id":{},"result":"D3wwzU6vvoUUYehv4qoFzq42DZnLoAETeFzeyk8swH4o"}}"#,
serde_json::to_string(&id).unwrap()
)
}
Expand Down
6 changes: 3 additions & 3 deletions tests/tests/proc_macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ async fn macro_optional_param_parsing() {
.raw_json_request(r#"{"jsonrpc":"2.0","method":"foo_optional_params","params":{"a":22,"c":50},"id":0}"#, 1)
.await
.unwrap();
assert_eq!(resp, r#"{"jsonrpc":"2.0","result":"Called with: 22, None, Some(50)","id":0}"#);
assert_eq!(resp, r#"{"jsonrpc":"2.0","id":0,"result":"Called with: 22, None, Some(50)"}"#);
}

#[tokio::test]
Expand All @@ -286,14 +286,14 @@ async fn macro_zero_copy_cow() {
.unwrap();

// std::borrow::Cow<str> always deserialized to owned variant here
assert_eq!(resp, r#"{"jsonrpc":"2.0","result":"Zero copy params: false","id":0}"#);
assert_eq!(resp, r#"{"jsonrpc":"2.0","id":0,"result":"Zero copy params: false"}"#);

// serde_json will have to allocate a new string to replace `\t` with byte 0x09 (tab)
let (resp, _) = module
.raw_json_request(r#"{"jsonrpc":"2.0","method":"foo_zero_copy_cow","params":["\tfoo"],"id":0}"#, 1)
.await
.unwrap();
assert_eq!(resp, r#"{"jsonrpc":"2.0","result":"Zero copy params: false","id":0}"#);
assert_eq!(resp, r#"{"jsonrpc":"2.0","id":0,"result":"Zero copy params: false"}"#);
}

// Disabled on MacOS as GH CI timings on Mac vary wildly (~100ms) making this test fail.
Expand Down
8 changes: 4 additions & 4 deletions tests/tests/rpc_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,13 @@ async fn subscribe_unsubscribe_without_server() {
let unsub_req = format!("{{\"jsonrpc\":\"2.0\",\"method\":\"my_unsub\",\"params\":[{}],\"id\":1}}", ser_id);
let (resp, _) = module.raw_json_request(&unsub_req, 1).await.unwrap();

assert_eq!(resp, r#"{"jsonrpc":"2.0","result":true,"id":1}"#);
assert_eq!(resp, r#"{"jsonrpc":"2.0","id":1,"result":true}"#);

// Unsubscribe already performed; should be error.
let unsub_req = format!("{{\"jsonrpc\":\"2.0\",\"method\":\"my_unsub\",\"params\":[{}],\"id\":1}}", ser_id);
let (resp, _) = module.raw_json_request(&unsub_req, 2).await.unwrap();

assert_eq!(resp, r#"{"jsonrpc":"2.0","result":false,"id":1}"#);
assert_eq!(resp, r#"{"jsonrpc":"2.0","id":1,"result":false}"#);
}

let sub1 = subscribe_and_assert(&module);
Expand Down Expand Up @@ -429,7 +429,7 @@ async fn reject_works() {
.unwrap();

let (rp, mut stream) = module.raw_json_request(r#"{"jsonrpc":"2.0","method":"my_sub","id":0}"#, 1).await.unwrap();
assert_eq!(rp, r#"{"jsonrpc":"2.0","error":{"code":-32700,"message":"rejected"},"id":0}"#);
assert_eq!(rp, r#"{"jsonrpc":"2.0","id":0,"error":{"code":-32700,"message":"rejected"}}"#);
assert!(stream.recv().await.is_none());
}

Expand Down Expand Up @@ -629,7 +629,7 @@ async fn method_response_notify_on_completion() {
// Low level call should also work.
let (rp, _) =
module.raw_json_request(r#"{"jsonrpc":"2.0","method":"hey","params":["success"],"id":0}"#, 1).await.unwrap();
assert_eq!(rp, r#"{"jsonrpc":"2.0","result":"lo","id":0}"#);
assert_eq!(rp, r#"{"jsonrpc":"2.0","id":0,"result":"lo"}"#);
assert!(matches!(rx.recv().await, Some(Ok(_))));

// Error call should return a failed notification.
Expand Down
Loading

0 comments on commit 87999cf

Please sign in to comment.