From c7e128eed1358c86bb2878b46a519ffab30144d5 Mon Sep 17 00:00:00 2001 From: Bahex Date: Wed, 6 Nov 2024 17:09:40 +0300 Subject: [PATCH] add table params support to `url join` and `url build-query` (#14239) Add `table` support to `url join` for the `params` field, and as input to `url build-query` #14162 # Description ```nushell { "scheme": "http", "username": "usr", "password": "pwd", "host": "localhost", "params": [ ["key", "value"]; ["par_1", "aaa"], ["par_2", "bbb"], ["par_1", "ccc"], ["par_2", "ddd"], ], "port": "1234", } | url join ``` ``` http://usr:pwd@localhost:1234?par_1=aaa&par_2=bbb&par_1=ccc&par_2=ddd ``` --- ```nushell [ ["key", "value"]; ["par_1", "aaa"], ["par_2", "bbb"], ["par_1", "ccc"], ["par_2", "ddd"], ] | url build-query ``` ``` par_1=aaa&par_2=bbb&par_1=ccc&par_2=ddd ``` # User-Facing Changes ## `url build-query` - can no longer accept one row table input as if it were a record --------- Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com> --- .../nu-command/src/network/url/build_query.rs | 56 ++++++------ crates/nu-command/src/network/url/join.rs | 73 +++++++-------- crates/nu-command/src/network/url/query.rs | 48 ++++++++++ crates/nu-command/tests/commands/url/join.rs | 88 ++++++++++++++++++- 4 files changed, 196 insertions(+), 69 deletions(-) diff --git a/crates/nu-command/src/network/url/build_query.rs b/crates/nu-command/src/network/url/build_query.rs index 11fd9aba94..e11f476469 100644 --- a/crates/nu-command/src/network/url/build_query.rs +++ b/crates/nu-command/src/network/url/build_query.rs @@ -1,6 +1,6 @@ use nu_engine::command_prelude::*; -use super::query::record_to_query_string; +use super::query::{record_to_query_string, table_to_query_string}; #[derive(Clone)] pub struct SubCommand; @@ -14,7 +14,10 @@ impl Command for SubCommand { Signature::build("url build-query") .input_output_types(vec![ (Type::record(), Type::String), - (Type::table(), Type::String), + ( + Type::Table([("key".into(), Type::Any), ("value".into(), Type::Any)].into()), + Type::String, + ), ]) .category(Category::Network) } @@ -34,11 +37,6 @@ impl Command for SubCommand { example: r#"{ mode:normal userid:31415 } | url build-query"#, result: Some(Value::test_string("mode=normal&userid=31415")), }, - Example { - description: "Outputs a query string representing the contents of this 1-row table", - example: r#"[[foo bar]; ["1" "2"]] | url build-query"#, - result: Some(Value::test_string("foo=1&bar=2")), - }, Example { description: "Outputs a query string representing the contents of this record, with a value that needs to be url-encoded", example: r#"{a:"AT&T", b: "AT T"} | url build-query"#, @@ -49,6 +47,11 @@ impl Command for SubCommand { example: r#"{a: ["one", "two"], b: "three"} | url build-query"#, result: Some(Value::test_string("a=one&a=two&b=three")), }, + Example { + description: "Outputs a query string representing the contents of this table containing key-value pairs", + example: r#"[[key, value]; [a, one], [a, two], [b, three], [a, four]] | url build-query"#, + result: Some(Value::test_string("a=one&a=two&b=three&a=four")), + }, ] } @@ -60,32 +63,25 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { let head = call.head; - to_url(input, head) + let input_span = input.span().unwrap_or(head); + let value = input.into_value(input_span)?; + let span = value.span(); + let output = match value { + Value::Record { ref val, .. } => record_to_query_string(val, span, head), + Value::List { ref vals, .. } => table_to_query_string(vals, span, head), + // Propagate existing errors + Value::Error { error, .. } => Err(*error), + other => Err(ShellError::UnsupportedInput { + msg: "Expected a record or table from pipeline".to_string(), + input: "value originates from here".into(), + msg_span: head, + input_span: other.span(), + }), + }; + Ok(Value::string(output?, head).into_pipeline_data()) } } -fn to_url(input: PipelineData, head: Span) -> Result { - let output: Result = input - .into_iter() - .map(move |value| { - let span = value.span(); - match value { - Value::Record { ref val, .. } => record_to_query_string(val, span, head), - // Propagate existing errors - Value::Error { error, .. } => Err(*error), - other => Err(ShellError::UnsupportedInput { - msg: "Expected a table from pipeline".to_string(), - input: "value originates from here".into(), - msg_span: head, - input_span: other.span(), - }), - } - }) - .collect(); - - Ok(Value::string(output?, head).into_pipeline_data()) -} - #[cfg(test)] mod test { use super::*; diff --git a/crates/nu-command/src/network/url/join.rs b/crates/nu-command/src/network/url/join.rs index 3b52b1d6b0..b8e120134a 100644 --- a/crates/nu-command/src/network/url/join.rs +++ b/crates/nu-command/src/network/url/join.rs @@ -1,6 +1,6 @@ use nu_engine::command_prelude::*; -use super::query::record_to_query_string; +use super::query::{record_to_query_string, table_to_query_string}; #[derive(Clone)] pub struct SubCommand; @@ -112,7 +112,7 @@ impl Command for SubCommand { .into_owned() .into_iter() .try_fold(UrlComponents::new(), |url, (k, v)| { - url.add_component(k, v, span, engine_state) + url.add_component(k, v, head, engine_state) }); url_components?.to_url(span) @@ -155,7 +155,7 @@ impl UrlComponents { self, key: String, value: Value, - span: Span, + head: Span, engine_state: &EngineState, ) -> Result { let value_span = value.span(); @@ -194,40 +194,41 @@ impl UrlComponents { } if key == "params" { - return match value { - Value::Record { ref val, .. } => { - let mut qs = record_to_query_string(val, value_span, span)?; - - qs = if !qs.trim().is_empty() { - format!("?{qs}") - } else { - qs - }; - - if let Some(q) = self.query { - if q != qs { - // if query is present it means that also query_span is set. - return Err(ShellError::IncompatibleParameters { - left_message: format!("Mismatch, qs from params is: {qs}"), - left_span: value_span, - right_message: format!("instead query is: {q}"), - right_span: self.query_span.unwrap_or(Span::unknown()), - }); - } - } - - Ok(Self { - query: Some(qs), - params_span: Some(value_span), - ..self + let mut qs = match value { + Value::Record { ref val, .. } => record_to_query_string(val, value_span, head)?, + Value::List { ref vals, .. } => table_to_query_string(vals, value_span, head)?, + Value::Error { error, .. } => return Err(*error), + other => { + return Err(ShellError::IncompatibleParametersSingle { + msg: String::from("Key params has to be a record or a table"), + span: other.span(), }) } - Value::Error { error, .. } => Err(*error), - other => Err(ShellError::IncompatibleParametersSingle { - msg: String::from("Key params has to be a record"), - span: other.span(), - }), }; + + qs = if !qs.trim().is_empty() { + format!("?{qs}") + } else { + qs + }; + + if let Some(q) = self.query { + if q != qs { + // if query is present it means that also query_span is set. + return Err(ShellError::IncompatibleParameters { + left_message: format!("Mismatch, query string from params is: {qs}"), + left_span: value_span, + right_message: format!("instead query is: {q}"), + right_span: self.query_span.unwrap_or(Span::unknown()), + }); + } + } + + return Ok(Self { + query: Some(qs), + params_span: Some(value_span), + ..self + }); } // apart from port and params all other keys are strings. @@ -267,7 +268,7 @@ impl UrlComponents { return Err(ShellError::IncompatibleParameters { left_message: format!("Mismatch, query param is: {s}"), left_span: value_span, - right_message: format!("instead qs from params is: {q}"), + right_message: format!("instead query string from params is: {q}"), right_span: self.params_span.unwrap_or(Span::unknown()), }); } @@ -293,7 +294,7 @@ impl UrlComponents { &ShellError::GenericError { error: format!("'{key}' is not a valid URL field"), msg: format!("remove '{key}' col from input record"), - span: Some(span), + span: Some(value_span), help: None, inner: vec![], }, diff --git a/crates/nu-command/src/network/url/query.rs b/crates/nu-command/src/network/url/query.rs index 21b9f541cb..9dee687cca 100644 --- a/crates/nu-command/src/network/url/query.rs +++ b/crates/nu-command/src/network/url/query.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use nu_protocol::{IntoValue, Record, ShellError, Span, Type, Value}; pub fn record_to_query_string( @@ -43,6 +45,52 @@ pub fn record_to_query_string( }) } +pub fn table_to_query_string( + table: &[Value], + span: Span, + head: Span, +) -> Result { + let row_vec = table + .iter() + .map(|val| match val { + Value::Record { val, internal_span } => key_value_from_record(val, *internal_span), + _ => Err(ShellError::UnsupportedInput { + msg: "expected a table".into(), + input: "not a table, contains non-record values".into(), + msg_span: head, + input_span: span, + }), + }) + .collect::, ShellError>>()?; + + serde_urlencoded::to_string(row_vec).map_err(|_| ShellError::CantConvert { + to_type: "URL".into(), + from_type: Type::table().to_string(), + span: head, + help: None, + }) +} + +fn key_value_from_record(record: &Record, span: Span) -> Result<(Cow, Cow), ShellError> { + let key = record + .get("key") + .ok_or_else(|| ShellError::CantFindColumn { + col_name: "key".into(), + span: None, + src_span: span, + })? + .coerce_str()?; + let value = record + .get("value") + .ok_or_else(|| ShellError::CantFindColumn { + col_name: "value".into(), + span: None, + src_span: span, + })? + .coerce_str()?; + Ok((key, value)) +} + pub fn query_string_to_table(query: &str, head: Span, span: Span) -> Result { let params = serde_urlencoded::from_str::>(query) .map_err(|_| ShellError::UnsupportedInput { diff --git a/crates/nu-command/tests/commands/url/join.rs b/crates/nu-command/tests/commands/url/join.rs index bafe7881f4..92257a41ff 100644 --- a/crates/nu-command/tests/commands/url/join.rs +++ b/crates/nu-command/tests/commands/url/join.rs @@ -156,7 +156,7 @@ fn url_join_with_different_query_and_params() { assert!(actual .err - .contains("Mismatch, qs from params is: ?par_1=aaab&par_2=bbb")); + .contains("Mismatch, query string from params is: ?par_1=aaab&par_2=bbb")); assert!(actual .err .contains("instead query is: ?par_1=aaa&par_2=bbb")); @@ -183,7 +183,7 @@ fn url_join_with_different_query_and_params() { .contains("Mismatch, query param is: par_1=aaa&par_2=bbb")); assert!(actual .err - .contains("instead qs from params is: ?par_1=aaab&par_2=bbb")); + .contains("instead query string from params is: ?par_1=aaab&par_2=bbb")); } #[test] @@ -201,7 +201,9 @@ fn url_join_with_invalid_params() { "# )); - assert!(actual.err.contains("Key params has to be a record")); + assert!(actual + .err + .contains("Key params has to be a record or a table")); } #[test] @@ -346,3 +348,83 @@ fn url_join_with_empty_params() { assert_eq!(actual.out, "https://localhost/foo"); } + +#[test] +fn url_join_with_list_in_params() { + let actual = nu!(pipeline( + r#" + { + "scheme": "http", + "username": "usr", + "password": "pwd", + "host": "localhost", + "params": { + "par_1": "aaa", + "par_2": ["bbb", "ccc"] + }, + "port": "1234", + } | url join + "# + )); + + assert_eq!( + actual.out, + "http://usr:pwd@localhost:1234?par_1=aaa&par_2=bbb&par_2=ccc" + ); +} + +#[test] +fn url_join_with_params_table() { + let actual = nu!(pipeline( + r#" + { + "scheme": "http", + "username": "usr", + "password": "pwd", + "host": "localhost", + "params": [ + ["key", "value"]; + ["par_1", "aaa"], + ["par_2", "bbb"], + ["par_1", "ccc"], + ["par_2", "ddd"], + ], + "port": "1234", + } | url join + "# + )); + + assert_eq!( + actual.out, + "http://usr:pwd@localhost:1234?par_1=aaa&par_2=bbb&par_1=ccc&par_2=ddd" + ); +} + +#[test] +fn url_join_with_params_invalid_table() { + let actual = nu!(pipeline( + r#" + { + "scheme": "http", + "username": "usr", + "password": "pwd", + "host": "localhost", + "params": ( + [ + ["key", "value"]; + ["par_1", "aaa"], + ["par_2", "bbb"], + ["par_1", "ccc"], + ["par_2", "ddd"], + ] ++ ["not a record"] + ), + "port": "1234", + } | url join + "# + )); + + assert!(actual.err.contains("expected a table")); + assert!(actual + .err + .contains("not a table, contains non-record values")); +}