From 4792328d0e20fdc72c76700ad4ba8ce9464b4063 Mon Sep 17 00:00:00 2001 From: xonas Date: Wed, 4 Sep 2024 18:05:39 -0300 Subject: [PATCH] Refactor send_request in client.rs (#13701) Closes #13687 Closes #13686 # Description Light refactoring of `send_request `in `client.rs`. In the end there are more lines but now the logic is more concise and facilitates adding new conditions in the future. Unit tests ran fine and I tested a few cases manually. Cool project btw, I'll be using nushell from now on. --- crates/nu-command/src/network/http/client.rs | 287 ++++++++++-------- .../tests/commands/network/http/post.rs | 30 ++ crates/nu-protocol/src/errors/shell_error.rs | 9 + 3 files changed, 203 insertions(+), 123 deletions(-) diff --git a/crates/nu-command/src/network/http/client.rs b/crates/nu-command/src/network/http/client.rs index 75ef8c4897..5c335fedf0 100644 --- a/crates/nu-command/src/network/http/client.rs +++ b/crates/nu-command/src/network/http/client.rs @@ -18,6 +18,8 @@ use std::{ use ureq::{Error, ErrorKind, Request, Response}; use url::Url; +const HTTP_DOCS: &str = "https://www.nushell.sh/cookbook/http.html"; + #[derive(PartialEq, Eq)] pub enum BodyType { Json, @@ -221,135 +223,174 @@ pub fn send_request( _ => (BodyType::Unknown, request), }; - match body { - Value::Binary { val, .. } => send_cancellable_request( - &request_url, - Box::new(move || req.send_bytes(&val)), - span, - signals, - ), - Value::String { .. } if body_type == BodyType::Json => { - let data = value_to_json_value(&body)?; - send_cancellable_request( - &request_url, - Box::new(|| req.send_json(data)), - span, - signals, - ) + match body_type { + BodyType::Json => send_json_request(&request_url, body, req, span, signals), + BodyType::Form => send_form_request(&request_url, body, req, span, signals), + BodyType::Multipart => { + send_multipart_request(&request_url, body, req, span, signals) } - Value::String { val, .. } => send_cancellable_request( - &request_url, - Box::new(move || req.send_string(&val)), - span, - signals, - ), - Value::Record { .. } if body_type == BodyType::Json => { - let data = value_to_json_value(&body)?; - send_cancellable_request( - &request_url, - Box::new(|| req.send_json(data)), - span, - signals, - ) - } - Value::Record { val, .. } if body_type == BodyType::Form => { - let mut data: Vec<(String, String)> = Vec::with_capacity(val.len()); - - for (col, val) in val.into_owned() { - data.push((col, val.coerce_into_string()?)) - } - - let request_fn = move || { - // coerce `data` into a shape that send_form() is happy with - let data = data - .iter() - .map(|(a, b)| (a.as_str(), b.as_str())) - .collect::>(); - req.send_form(&data) - }; - send_cancellable_request(&request_url, Box::new(request_fn), span, signals) - } - // multipart form upload - Value::Record { val, .. } if body_type == BodyType::Multipart => { - let mut builder = MultipartWriter::new(); - - let err = |e| { - ShellErrorOrRequestError::ShellError(ShellError::IOError { - msg: format!("failed to build multipart data: {}", e), - }) - }; - - for (col, val) in val.into_owned() { - if let Value::Binary { val, .. } = val { - let headers = [ - "Content-Type: application/octet-stream".to_string(), - "Content-Transfer-Encoding: binary".to_string(), - format!( - "Content-Disposition: form-data; name=\"{}\"; filename=\"{}\"", - col, col - ), - format!("Content-Length: {}", val.len()), - ]; - builder - .add(&mut Cursor::new(val), &headers.join("\n")) - .map_err(err)?; - } else { - let headers = - format!(r#"Content-Disposition: form-data; name="{}""#, col); - builder - .add(val.coerce_into_string()?.as_bytes(), &headers) - .map_err(err)?; - } - } - builder.finish(); - - let (boundary, data) = (builder.boundary, builder.data); - let content_type = format!("multipart/form-data; boundary={}", boundary); - - let request_fn = - move || req.set("Content-Type", &content_type).send_bytes(&data); - - send_cancellable_request(&request_url, Box::new(request_fn), span, signals) - } - Value::List { vals, .. } if body_type == BodyType::Form => { - if vals.len() % 2 != 0 { - return Err(ShellErrorOrRequestError::ShellError(ShellError::IOError { - msg: "unsupported body input".into(), - })); - } - - let data = vals - .chunks(2) - .map(|it| Ok((it[0].coerce_string()?, it[1].coerce_string()?))) - .collect::, ShellErrorOrRequestError>>()?; - - let request_fn = move || { - // coerce `data` into a shape that send_form() is happy with - let data = data - .iter() - .map(|(a, b)| (a.as_str(), b.as_str())) - .collect::>(); - req.send_form(&data) - }; - send_cancellable_request(&request_url, Box::new(request_fn), span, signals) - } - Value::List { .. } if body_type == BodyType::Json => { - let data = value_to_json_value(&body)?; - send_cancellable_request( - &request_url, - Box::new(|| req.send_json(data)), - span, - signals, - ) - } - _ => Err(ShellErrorOrRequestError::ShellError(ShellError::IOError { - msg: "unsupported body input".into(), - })), + BodyType::Unknown => send_default_request(&request_url, body, req, span, signals), } } } } +fn send_json_request( + request_url: &str, + body: Value, + req: Request, + span: Span, + signals: &Signals, +) -> Result { + let data = match body { + Value::Int { .. } | Value::List { .. } | Value::String { .. } | Value::Record { .. } => { + value_to_json_value(&body)? + } + _ => { + return Err(ShellErrorOrRequestError::ShellError( + ShellError::UnsupportedHttpBody { + msg: format!("Accepted types: [Int, List, String, Record]. Check: {HTTP_DOCS}"), + }, + )) + } + }; + send_cancellable_request(request_url, Box::new(|| req.send_json(data)), span, signals) +} + +fn send_form_request( + request_url: &str, + body: Value, + req: Request, + span: Span, + signals: &Signals, +) -> Result { + let build_request_fn = |data: Vec<(String, String)>| { + // coerce `data` into a shape that send_form() is happy with + let data = data + .iter() + .map(|(a, b)| (a.as_str(), b.as_str())) + .collect::>(); + req.send_form(&data) + }; + + match body { + Value::List { vals, .. } => { + if vals.len() % 2 != 0 { + return Err(ShellErrorOrRequestError::ShellError(ShellError::UnsupportedHttpBody { + msg: "Body type 'List' for form requests requires paired values. E.g.: [value, 10]".into(), + })); + } + + let data = vals + .chunks(2) + .map(|it| Ok((it[0].coerce_string()?, it[1].coerce_string()?))) + .collect::, ShellErrorOrRequestError>>()?; + + let request_fn = Box::new(|| build_request_fn(data)); + send_cancellable_request(request_url, request_fn, span, signals) + } + Value::Record { val, .. } => { + let mut data: Vec<(String, String)> = Vec::with_capacity(val.len()); + + for (col, val) in val.into_owned() { + data.push((col, val.coerce_into_string()?)) + } + + let request_fn = Box::new(|| build_request_fn(data)); + send_cancellable_request(request_url, request_fn, span, signals) + } + _ => Err(ShellErrorOrRequestError::ShellError( + ShellError::UnsupportedHttpBody { + msg: format!("Accepted types: [List, Record]. Check: {HTTP_DOCS}"), + }, + )), + } +} + +fn send_multipart_request( + request_url: &str, + body: Value, + req: Request, + span: Span, + signals: &Signals, +) -> Result { + let request_fn = match body { + Value::Record { val, .. } => { + let mut builder = MultipartWriter::new(); + + let err = |e| { + ShellErrorOrRequestError::ShellError(ShellError::IOError { + msg: format!("failed to build multipart data: {}", e), + }) + }; + + for (col, val) in val.into_owned() { + if let Value::Binary { val, .. } = val { + let headers = [ + "Content-Type: application/octet-stream".to_string(), + "Content-Transfer-Encoding: binary".to_string(), + format!( + "Content-Disposition: form-data; name=\"{}\"; filename=\"{}\"", + col, col + ), + format!("Content-Length: {}", val.len()), + ]; + builder + .add(&mut Cursor::new(val), &headers.join("\n")) + .map_err(err)?; + } else { + let headers = format!(r#"Content-Disposition: form-data; name="{}""#, col); + builder + .add(val.coerce_into_string()?.as_bytes(), &headers) + .map_err(err)?; + } + } + builder.finish(); + + let (boundary, data) = (builder.boundary, builder.data); + let content_type = format!("multipart/form-data; boundary={}", boundary); + + move || req.set("Content-Type", &content_type).send_bytes(&data) + } + _ => { + return Err(ShellErrorOrRequestError::ShellError( + ShellError::UnsupportedHttpBody { + msg: format!("Accepted types: [Record]. Check: {HTTP_DOCS}"), + }, + )) + } + }; + send_cancellable_request(request_url, Box::new(request_fn), span, signals) +} + +fn send_default_request( + request_url: &str, + body: Value, + req: Request, + span: Span, + signals: &Signals, +) -> Result { + match body { + Value::Binary { val, .. } => send_cancellable_request( + request_url, + Box::new(move || req.send_bytes(&val)), + span, + signals, + ), + Value::String { val, .. } => send_cancellable_request( + request_url, + Box::new(move || req.send_string(&val)), + span, + signals, + ), + _ => Err(ShellErrorOrRequestError::ShellError( + ShellError::UnsupportedHttpBody { + msg: format!("Accepted types: [Binary, String]. Check: {HTTP_DOCS}"), + }, + )), + } +} + // Helper method used to make blocking HTTP request calls cancellable with ctrl+c // ureq functions can block for a long time (default 30s?) while attempting to make an HTTP connection fn send_cancellable_request( diff --git a/crates/nu-command/tests/commands/network/http/post.rs b/crates/nu-command/tests/commands/network/http/post.rs index 11db87168e..6fbcb03b69 100644 --- a/crates/nu-command/tests/commands/network/http/post.rs +++ b/crates/nu-command/tests/commands/network/http/post.rs @@ -133,6 +133,36 @@ fn http_post_json_list_is_success() { assert!(actual.out.is_empty()) } +#[test] +fn http_post_json_int_is_success() { + let mut server = Server::new(); + + let mock = server.mock("POST", "/").match_body(r#"50"#).create(); + + let actual = nu!(format!( + r#"http post -t 'application/json' {url} 50"#, + url = server.url() + )); + + mock.assert(); + assert!(actual.out.is_empty()) +} + +#[test] +fn http_post_json_string_is_success() { + let mut server = Server::new(); + + let mock = server.mock("POST", "/").match_body(r#""test""#).create(); + + let actual = nu!(format!( + r#"http post -t 'application/json' {url} "test""#, + url = server.url() + )); + + mock.assert(); + assert!(actual.out.is_empty()) +} + #[test] fn http_post_follows_redirect() { let mut server = Server::new(); diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index ab01ccfa54..b4224ae05c 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -639,6 +639,15 @@ pub enum ShellError { span: Span, }, + /// An unsupported body input was used for the respective application body type in 'http' command + /// + /// ## Resolution + /// + /// This error is fairly generic. Refer to the specific error message for further details. + #[error("Unsupported body for current content type")] + #[diagnostic(code(nu::shell::unsupported_body), help("{msg}"))] + UnsupportedHttpBody { msg: String }, + /// An operation was attempted with an input unsupported for some reason. /// /// ## Resolution