From d34a24db333126f11db81ef6f58fcffe1cd07785 Mon Sep 17 00:00:00 2001 From: Jack Wright <56345+ayax79@users.noreply.github.com> Date: Thu, 1 Aug 2024 02:10:52 -0700 Subject: [PATCH] setting content type metadata on all core `to *` commands (#13506) # Description All core `to *` set content type pipeline metadata. # User-Facing Changes - For consistency, `from json` no longer sets the content type metadata # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib --- crates/nu-cmd-lang/src/example_support.rs | 2 +- crates/nu-command/src/formats/from/json.rs | 18 ++----- crates/nu-command/src/formats/to/csv.rs | 51 ++++++++++++++++++- crates/nu-command/src/formats/to/delimited.rs | 32 +++++++++--- crates/nu-command/src/formats/to/json.rs | 36 ++++++++++++- crates/nu-command/src/formats/to/md.rs | 45 +++++++++++++++- crates/nu-command/src/formats/to/msgpack.rs | 43 +++++++++++++++- crates/nu-command/src/formats/to/nuon.rs | 47 +++++++++++++++-- crates/nu-command/src/formats/to/text.rs | 38 +++++++++++++- crates/nu-command/src/formats/to/tsv.rs | 51 ++++++++++++++++++- crates/nu-command/src/formats/to/xml.rs | 40 ++++++++++++++- crates/nu-command/src/formats/to/yaml.rs | 45 +++++++++++++++- 12 files changed, 411 insertions(+), 37 deletions(-) diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index 72ea36972f..70ee7a9b0f 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -72,7 +72,7 @@ pub fn check_example_input_and_output_types_match_command_signature( witnessed_type_transformations } -fn eval_pipeline_without_terminal_expression( +pub fn eval_pipeline_without_terminal_expression( src: &str, cwd: &std::path::Path, engine_state: &mut Box, diff --git a/crates/nu-command/src/formats/from/json.rs b/crates/nu-command/src/formats/from/json.rs index 6252afff4c..6d46df78a3 100644 --- a/crates/nu-command/src/formats/from/json.rs +++ b/crates/nu-command/src/formats/from/json.rs @@ -1,7 +1,7 @@ use std::io::{BufRead, Cursor}; use nu_engine::command_prelude::*; -use nu_protocol::{ListStream, PipelineMetadata, Signals}; +use nu_protocol::{ListStream, Signals}; #[derive(Clone)] pub struct FromJson; @@ -83,7 +83,7 @@ impl Command for FromJson { strict, engine_state.signals().clone(), ), - update_metadata(metadata), + metadata, )) } PipelineData::ByteStream(stream, metadata) @@ -92,7 +92,7 @@ impl Command for FromJson { if let Some(reader) = stream.reader() { Ok(PipelineData::ListStream( read_json_lines(reader, span, strict, Signals::empty()), - update_metadata(metadata), + metadata, )) } else { Ok(PipelineData::Empty) @@ -115,10 +115,10 @@ impl Command for FromJson { if strict { Ok(convert_string_to_value_strict(&string_input, span)? - .into_pipeline_data_with_metadata(update_metadata(metadata))) + .into_pipeline_data_with_metadata(metadata)) } else { Ok(convert_string_to_value(&string_input, span)? - .into_pipeline_data_with_metadata(update_metadata(metadata))) + .into_pipeline_data_with_metadata(metadata)) } } } @@ -265,14 +265,6 @@ fn convert_string_to_value_strict(string_input: &str, span: Span) -> Result) -> Option { - metadata - .map(|md| md.with_content_type(Some("application/json".into()))) - .or_else(|| { - Some(PipelineMetadata::default().with_content_type(Some("application/json".into()))) - }) -} - #[cfg(test)] mod test { use super::*; diff --git a/crates/nu-command/src/formats/to/csv.rs b/crates/nu-command/src/formats/to/csv.rs index e7786e60cc..c5847abd6a 100644 --- a/crates/nu-command/src/formats/to/csv.rs +++ b/crates/nu-command/src/formats/to/csv.rs @@ -4,6 +4,8 @@ use crate::formats::to::delimited::to_delimited_data; use nu_engine::command_prelude::*; use nu_protocol::Config; +use super::delimited::ToDelimitedDataArgs; + #[derive(Clone)] pub struct ToCsv; @@ -116,17 +118,62 @@ fn to_csv( }, }; - to_delimited_data(noheaders, sep, columns, "CSV", input, head, config) + to_delimited_data( + ToDelimitedDataArgs { + noheaders, + separator: sep, + columns, + format_name: "CSV", + input, + head, + content_type: Some(mime::TEXT_CSV.to_string()), + }, + config, + ) } #[cfg(test)] mod test { + + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + use super::*; #[test] fn test_examples() { use crate::test_examples; - test_examples(ToCsv {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToCsv {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to csv | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("content_type" => Value::test_string("text/csv"))), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/delimited.rs b/crates/nu-command/src/formats/to/delimited.rs index 34bb06b8a2..ac04739a3d 100644 --- a/crates/nu-command/src/formats/to/delimited.rs +++ b/crates/nu-command/src/formats/to/delimited.rs @@ -69,18 +69,36 @@ fn make_cant_convert_error(value: &Value, format_name: &'static str) -> ShellErr } } +pub struct ToDelimitedDataArgs { + pub noheaders: bool, + pub separator: Spanned, + pub columns: Option>, + pub format_name: &'static str, + pub input: PipelineData, + pub head: Span, + pub content_type: Option, +} + pub fn to_delimited_data( - noheaders: bool, - separator: Spanned, - columns: Option>, - format_name: &'static str, - input: PipelineData, - head: Span, + ToDelimitedDataArgs { + noheaders, + separator, + columns, + format_name, + input, + head, + content_type, + }: ToDelimitedDataArgs, config: Arc, ) -> Result { let mut input = input; let span = input.span().unwrap_or(head); - let metadata = input.metadata(); + let metadata = Some( + input + .metadata() + .unwrap_or_default() + .with_content_type(content_type), + ); let separator = u8::try_from(separator.item).map_err(|_| ShellError::IncorrectValue { msg: "separator must be an ASCII character".into(), diff --git a/crates/nu-command/src/formats/to/json.rs b/crates/nu-command/src/formats/to/json.rs index 5722387e2c..729fe2293f 100644 --- a/crates/nu-command/src/formats/to/json.rs +++ b/crates/nu-command/src/formats/to/json.rs @@ -64,7 +64,7 @@ impl Command for ToJson { let res = Value::string(serde_json_string, span); let metadata = PipelineMetadata { data_source: nu_protocol::DataSource::None, - content_type: Some("application/json".to_string()), + content_type: Some(mime::APPLICATION_JSON.to_string()), }; Ok(PipelineData::Value(res, Some(metadata))) } @@ -159,6 +159,10 @@ fn json_list(input: &[Value]) -> Result, ShellError> { #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + use super::*; #[test] @@ -167,4 +171,34 @@ mod test { test_examples(ToJson {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToJson {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to json | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("content_type" => Value::test_string("application/json"))), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/md.rs b/crates/nu-command/src/formats/to/md.rs index 737a328cd3..7f86c5eb06 100644 --- a/crates/nu-command/src/formats/to/md.rs +++ b/crates/nu-command/src/formats/to/md.rs @@ -82,6 +82,12 @@ fn to_md( config: &Config, head: Span, ) -> Result { + // text/markdown became a valid mimetype with rfc7763 + let metadata = input + .metadata() + .unwrap_or_default() + .with_content_type(Some("text/markdown".into())); + let (grouped_input, single_list) = group_by(input, head, config); if per_element || single_list { return Ok(Value::string( @@ -95,9 +101,10 @@ fn to_md( .join(""), head, ) - .into_pipeline_data()); + .into_pipeline_data_with_metadata(Some(metadata))); } - Ok(Value::string(table(grouped_input, pretty, config), head).into_pipeline_data()) + Ok(Value::string(table(grouped_input, pretty, config), head) + .into_pipeline_data_with_metadata(Some(metadata))) } fn fragment(input: Value, pretty: bool, config: &Config) -> String { @@ -328,7 +335,10 @@ fn get_padded_string(text: String, desired_length: usize, padding_character: cha #[cfg(test)] mod tests { + use crate::Metadata; + use super::*; + use nu_cmd_lang::eval_pipeline_without_terminal_expression; use nu_protocol::{record, Config, IntoPipelineData, Value}; fn one(string: &str) -> String { @@ -453,4 +463,35 @@ mod tests { "#) ); } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let state_delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToMd {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + let delta = state_delta; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to md | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("content_type" => Value::test_string("text/markdown"))), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/msgpack.rs b/crates/nu-command/src/formats/to/msgpack.rs index 02515e26fe..049b4de86d 100644 --- a/crates/nu-command/src/formats/to/msgpack.rs +++ b/crates/nu-command/src/formats/to/msgpack.rs @@ -74,13 +74,18 @@ MessagePack: https://msgpack.org/ call: &Call, input: PipelineData, ) -> Result { + let metadata = input + .metadata() + .unwrap_or_default() + .with_content_type(Some("application/x-msgpack".into())); + let value_span = input.span().unwrap_or(call.head); let value = input.into_value(value_span)?; let mut out = vec![]; write_value(&mut out, &value, 0)?; - Ok(Value::binary(out, call.head).into_pipeline_data()) + Ok(Value::binary(out, call.head).into_pipeline_data_with_metadata(Some(metadata))) } } @@ -268,6 +273,10 @@ where #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + use super::*; #[test] @@ -276,4 +285,36 @@ mod test { test_examples(ToMsgpack {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToMsgpack {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to msgpack | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record( + record!("content_type" => Value::test_string("application/x-msgpack")) + ), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/nuon.rs b/crates/nu-command/src/formats/to/nuon.rs index f40b7b5c1d..809ddc01ad 100644 --- a/crates/nu-command/src/formats/to/nuon.rs +++ b/crates/nu-command/src/formats/to/nuon.rs @@ -42,6 +42,11 @@ impl Command for ToNuon { call: &Call, input: PipelineData, ) -> Result { + let metadata = input + .metadata() + .unwrap_or_default() + .with_content_type(Some("application/x-nuon".into())); + let style = if call.has_flag(engine_state, stack, "raw")? { nuon::ToStyle::Raw } else if let Some(t) = call.get_flag(engine_state, stack, "tabs")? { @@ -56,9 +61,8 @@ impl Command for ToNuon { let value = input.into_value(span)?; match nuon::to_nuon(&value, style, Some(span)) { - Ok(serde_nuon_string) => { - Ok(Value::string(serde_nuon_string, span).into_pipeline_data()) - } + Ok(serde_nuon_string) => Ok(Value::string(serde_nuon_string, span) + .into_pipeline_data_with_metadata(Some(metadata))), _ => Ok(Value::error( ShellError::CantConvert { to_type: "NUON".into(), @@ -68,7 +72,7 @@ impl Command for ToNuon { }, span, ) - .into_pipeline_data()), + .into_pipeline_data_with_metadata(Some(metadata))), } } @@ -100,10 +104,45 @@ impl Command for ToNuon { #[cfg(test)] mod test { + use super::*; + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + #[test] fn test_examples() { use super::ToNuon; use crate::test_examples; test_examples(ToNuon {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToNuon {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to nuon | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("content_type" => Value::test_string("application/x-nuon"))), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/text.rs b/crates/nu-command/src/formats/to/text.rs index 33ca19f8f2..111c42cdff 100644 --- a/crates/nu-command/src/formats/to/text.rs +++ b/crates/nu-command/src/formats/to/text.rs @@ -134,14 +134,18 @@ fn local_into_string(value: Value, separator: &str, config: &Config) -> String { fn update_metadata(metadata: Option) -> Option { metadata - .map(|md| md.with_content_type(Some("text/plain".to_string()))) + .map(|md| md.with_content_type(Some(mime::TEXT_PLAIN.to_string()))) .or_else(|| { - Some(PipelineMetadata::default().with_content_type(Some("text/plain".to_string()))) + Some(PipelineMetadata::default().with_content_type(Some(mime::TEXT_PLAIN.to_string()))) }) } #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + use super::*; #[test] @@ -150,4 +154,34 @@ mod test { test_examples(ToText {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToText {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to text | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("content_type" => Value::test_string("text/plain"))), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/tsv.rs b/crates/nu-command/src/formats/to/tsv.rs index edc9592409..63f72f4584 100644 --- a/crates/nu-command/src/formats/to/tsv.rs +++ b/crates/nu-command/src/formats/to/tsv.rs @@ -4,6 +4,8 @@ use crate::formats::to::delimited::to_delimited_data; use nu_engine::command_prelude::*; use nu_protocol::Config; +use super::delimited::ToDelimitedDataArgs; + #[derive(Clone)] pub struct ToTsv; @@ -82,11 +84,26 @@ fn to_tsv( item: '\t', span: head, }; - to_delimited_data(noheaders, sep, columns, "TSV", input, head, config) + to_delimited_data( + ToDelimitedDataArgs { + noheaders, + separator: sep, + columns, + format_name: "TSV", + input, + head, + content_type: Some(mime::TEXT_TAB_SEPARATED_VALUES.to_string()), + }, + config, + ) } #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + use super::*; #[test] @@ -95,4 +112,36 @@ mod test { test_examples(ToTsv {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToTsv {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to tsv | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record( + record!("content_type" => Value::test_string("text/tab-separated-values")) + ), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/xml.rs b/crates/nu-command/src/formats/to/xml.rs index 648094318b..1285c558ee 100644 --- a/crates/nu-command/src/formats/to/xml.rs +++ b/crates/nu-command/src/formats/to/xml.rs @@ -132,6 +132,10 @@ impl Job { } fn run(mut self, input: PipelineData, head: Span) -> Result { + let metadata = input + .metadata() + .unwrap_or_default() + .with_content_type(Some("application/xml".into())); let value = input.into_value(head)?; self.write_xml_entry(value, true).and_then(|_| { @@ -141,7 +145,7 @@ impl Job { } else { return Err(ShellError::NonUtf8 { span: head }); }; - Ok(Value::string(s, head).into_pipeline_data()) + Ok(Value::string(s, head).into_pipeline_data_with_metadata(Some(metadata))) }) } @@ -508,6 +512,10 @@ impl Job { #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + use super::*; #[test] @@ -516,4 +524,34 @@ mod test { test_examples(ToXml {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToXml {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{tag: note attributes: {} content : [{tag: remember attributes: {} content : [{tag: null attributes: null content : Event}]}]} | to xml | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("content_type" => Value::test_string("application/xml"))), + result.expect("There should be a result") + ); + } } diff --git a/crates/nu-command/src/formats/to/yaml.rs b/crates/nu-command/src/formats/to/yaml.rs index bea2dd3381..c1693cda7d 100644 --- a/crates/nu-command/src/formats/to/yaml.rs +++ b/crates/nu-command/src/formats/to/yaml.rs @@ -95,11 +95,18 @@ pub fn value_to_yaml_value(v: &Value) -> Result { } fn to_yaml(input: PipelineData, head: Span) -> Result { + let metadata = input + .metadata() + .unwrap_or_default() + .with_content_type(Some("application/yaml".into())); let value = input.into_value(head)?; let yaml_value = value_to_yaml_value(&value)?; match serde_yaml::to_string(&yaml_value) { - Ok(serde_yaml_string) => Ok(Value::string(serde_yaml_string, head).into_pipeline_data()), + Ok(serde_yaml_string) => { + Ok(Value::string(serde_yaml_string, head) + .into_pipeline_data_with_metadata(Some(metadata))) + } _ => Ok(Value::error( ShellError::CantConvert { to_type: "YAML".into(), @@ -109,12 +116,16 @@ fn to_yaml(input: PipelineData, head: Span) -> Result }, head, ) - .into_pipeline_data()), + .into_pipeline_data_with_metadata(Some(metadata))), } } #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::Metadata; + use super::*; #[test] @@ -123,4 +134,34 @@ mod test { test_examples(ToYaml {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + // Base functions that are needed for testing + // Try to keep this working set small to keep tests running as fast as possible + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToYaml {})); + working_set.add_decl(Box::new(Metadata {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = "{a: 1 b: 2} | to yaml | metadata | get content_type"; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("content_type" => Value::test_string("application/yaml"))), + result.expect("There should be a result") + ); + } }