add table params support to url join and url build-query (nushell#14239)

Add `table<key, value>` support to `url join` for the `params` field,
and as input to `url build-query` nushell#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 <[email protected]>

Author: 2 people

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<PipelineData, ShellError> {
         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<PipelineData, ShellError> {
-    let output: Result<String, ShellError> = 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::*;

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<Self, ShellError> {
         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![],
                     },

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<String, ShellError> {
+    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::<Result<Vec<_>, 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<str>, Cow<str>), 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<Value, ShellError> {
     let params = serde_urlencoded::from_str::<Vec<(String, String)>>(query)
         .map_err(|_| ShellError::UnsupportedInput {

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