From 88fd9de523eb75af12591baf05084b89217a391e Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Thu, 21 Nov 2024 22:48:18 -0800 Subject: [PATCH 01/11] print native query info after writing configuration --- Cargo.lock | 24 ++++ crates/cli/Cargo.toml | 5 +- crates/cli/src/native_query/mod.rs | 6 +- .../cli/src/native_query/pretty_printing.rs | 132 ++++++++++++++++++ crates/configuration/src/native_query.rs | 9 ++ 5 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 crates/cli/src/native_query/pretty_printing.rs diff --git a/Cargo.lock b/Cargo.lock index b682383..b003a9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -109,6 +109,12 @@ version = "1.0.86" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b3d1d046238990b9cf5bcde22a3fb3584ee5cf65fb2765f454ed428c7a0063da" +[[package]] +name = "arrayvec" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23b62fc65de8e4e7f52534fb52b0f3ed04746ae267519eef2a83941e8085068b" + [[package]] name = "assert_json" version = "0.1.0" @@ -1816,6 +1822,7 @@ dependencies = [ "ndc-models", "nom", "nonempty", + "pretty", "pretty_assertions", "proptest", "ref-cast", @@ -2345,6 +2352,17 @@ dependencies = [ "termtree", ] +[[package]] +name = "pretty" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b55c4d17d994b637e2f4daf6e5dc5d660d209d5642377d675d7a1c3ab69fa579" +dependencies = [ + "arrayvec", + "typed-arena", + "unicode-width", +] + [[package]] name = "pretty_assertions" version = "1.4.0" @@ -3617,6 +3635,12 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "typed-arena" +version = "2.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6af6ae20167a9ece4bcb41af5b80f8a1f1df981f6391189ce00fd257af04126a" + [[package]] name = "typed-builder" version = "0.10.0" diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 64fcfca..169b6b6 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -4,7 +4,7 @@ edition = "2021" version.workspace = true [features] -native-query-subcommand = [] +native-query-subcommand = ["dep:pretty", "dep:nom"] [dependencies] configuration = { path = "../configuration" } @@ -19,8 +19,9 @@ futures-util = "0.3.28" indexmap = { workspace = true } itertools = { workspace = true } ndc-models = { workspace = true } -nom = "^7.1.3" +nom = { version = "^7.1.3", optional = true } nonempty = "^0.10.0" +pretty = { version = "^0.12.3", optional = true } ref-cast = { workspace = true } regex = "^1.11.1" serde = { workspace = true } diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 56d3f08..13034cf 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -3,6 +3,7 @@ pub mod error; mod helpers; mod pipeline; mod pipeline_type_context; +mod pretty_printing; mod prune_object_types; mod reference_shorthand; mod type_annotation; @@ -30,6 +31,7 @@ use crate::Context; use self::error::Result; use self::pipeline::infer_pipeline_types; +use self::pretty_printing::pretty_print_native_query_info; /// Create native queries - custom MongoDB queries that integrate into your data graph #[derive(Clone, Debug, Subcommand)] @@ -124,9 +126,11 @@ pub async fn run(context: &Context, command: Command) -> anyhow::Result<()> { exit(ExitCode::ErrorWriting.into()) }; eprintln!( - "Wrote native query configuration to {}", + "\nWrote native query configuration to {}", native_query_path.to_string_lossy() ); + eprintln!(); + pretty_print_native_query_info(&mut std::io::stderr(), &native_query.value)?; Ok(()) } } diff --git a/crates/cli/src/native_query/pretty_printing.rs b/crates/cli/src/native_query/pretty_printing.rs new file mode 100644 index 0000000..b57fc63 --- /dev/null +++ b/crates/cli/src/native_query/pretty_printing.rs @@ -0,0 +1,132 @@ +use configuration::{schema::ObjectType, serialized::NativeQuery}; +use itertools::Itertools; +use pretty::{BoxAllocator, DocAllocator, DocBuilder, Pretty}; + +pub fn pretty_print_native_query_info( + output: &mut impl std::io::Write, + native_query: &NativeQuery, +) -> std::io::Result<()> { + let allocator = BoxAllocator; + native_query_info_printer::<_, ()>(native_query, &allocator) + .1 + .render(80, output)?; + Ok(()) +} + +fn native_query_info_printer<'a, D, A>( + nq: &'a NativeQuery, + allocator: &'a D, +) -> DocBuilder<'a, D, A> +where + D: DocAllocator<'a, A>, + D::Doc: Clone, + A: Clone, +{ + let input_collection = nq.input_collection.as_ref().map(|collection| { + allocator + .text("input collection: ") + .append(allocator.text(collection.to_string())) + }); + + let representation = Some( + allocator + .text("representation: ") + .append(allocator.text(nq.representation.to_str())), + ); + + let parameters = if !nq.arguments.is_empty() { + let params = nq.arguments.iter().map(|(name, definition)| { + allocator + .text(format!("{name}: ")) + .append(allocator.text(format!("{}", definition.r#type))) + }); + Some(section( + "parameters", + allocator.intersperse(params, allocator.line()), + allocator, + )) + } else { + None + }; + + let result_type = { + let body = if let Some(object_type) = nq.object_types.get(&nq.result_document_type) { + object_type_printer(object_type, allocator) + } else { + allocator.text(nq.result_document_type.to_string()) + }; + Some(section("result type", body, allocator)) + }; + + let other_object_types = nq + .object_types + .iter() + .filter(|(name, _)| **name != nq.result_document_type) + .collect_vec(); + let object_types_doc = if !other_object_types.is_empty() { + let docs = other_object_types.into_iter().map(|(name, definition)| { + allocator + .text(format!("{name} ")) + .append(object_type_printer(definition, allocator)) + }); + let separator = allocator.line().append(allocator.line()); + Some(section( + "object type definitions", + allocator.intersperse(docs, separator), + allocator, + )) + } else { + None + }; + + allocator.intersperse( + [ + input_collection, + representation, + parameters, + result_type, + object_types_doc, + ] + .into_iter() + .filter(Option::is_some), + allocator.hardline(), + ) +} + +fn object_type_printer<'a, D, A>(ot: &'a ObjectType, allocator: &'a D) -> DocBuilder<'a, D, A> +where + D: DocAllocator<'a, A>, + D::Doc: Clone, + A: Clone, +{ + let fields = ot.fields.iter().map(|(name, definition)| { + allocator + .text(format!("{name}: ")) + .append(allocator.text(format!("{}", definition.r#type))) + }); + let separator = allocator.text(",").append(allocator.line()); + let body = allocator.intersperse(fields, separator); + body.indent(2).enclose( + allocator.text("{").append(allocator.line()), + allocator.line().append(allocator.text("}")), + ) +} + +fn section<'a, D, A>( + heading: &'a str, + body: impl Pretty<'a, D, A>, + allocator: &'a D, +) -> DocBuilder<'a, D, A> +where + D: DocAllocator<'a, A>, + D::Doc: Clone, + A: Clone, +{ + let heading_doc = allocator.text("## ").append(heading); + allocator + .line() + .append(heading_doc) + .append(allocator.line()) + .append(allocator.line()) + .append(body) +} diff --git a/crates/configuration/src/native_query.rs b/crates/configuration/src/native_query.rs index 2b81999..9588e3f 100644 --- a/crates/configuration/src/native_query.rs +++ b/crates/configuration/src/native_query.rs @@ -45,3 +45,12 @@ pub enum NativeQueryRepresentation { Collection, Function, } + +impl NativeQueryRepresentation { + pub fn to_str(&self) -> &'static str { + match self { + NativeQueryRepresentation::Collection => "collection", + NativeQueryRepresentation::Function => "function", + } + } +} From bc41a868fb53036efe3ab36c53a8c6db51db11a9 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 09:34:53 -0800 Subject: [PATCH 02/11] add list subcommand --- crates/cli/src/native_query/mod.rs | 160 +++++++++++++++++------------ 1 file changed, 93 insertions(+), 67 deletions(-) diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 13034cf..ea894c0 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -53,6 +53,9 @@ pub enum Command { /// Path to a JSON file with an aggregation pipeline pipeline_path: PathBuf, }, + + /// List all configured native queries + List, } pub async fn run(context: &Context, command: Command) -> anyhow::Result<()> { @@ -62,78 +65,101 @@ pub async fn run(context: &Context, command: Command) -> anyhow::Result<()> { collection, force, pipeline_path, - } => { - let native_query_path = { - let path = get_native_query_path(context, &name); - if !force && fs::try_exists(&path).await? { - eprintln!( - "A native query named {name} already exists at {}.", - path.to_string_lossy() - ); - eprintln!("Re-run with --force to overwrite."); - exit(ExitCode::RefusedToOverwrite.into()) - } - path - }; - - let configuration = match read_directory_with_ignored_configs( - &context.path, - &[native_query_path.clone()], - ) - .await - { - Ok(c) => c, - Err(err) => { - eprintln!("Could not read connector configuration - configuration must be initialized before creating native queries.\n\n{err:#}"); - exit(ExitCode::CouldNotReadConfiguration.into()) - } - }; - eprintln!( - "Read configuration from {}", - &context.path.to_string_lossy() - ); + } => create(context, name, collection, force, &pipeline_path).await, + Command::List => list(context).await, + } +} - let pipeline = match read_pipeline(&pipeline_path).await { - Ok(p) => p, - Err(err) => { - eprintln!("Could not read aggregation pipeline.\n\n{err}"); - exit(ExitCode::CouldNotReadAggregationPipeline.into()) - } - }; - let native_query = - match native_query_from_pipeline(&configuration, &name, collection, pipeline) { - Ok(q) => WithName::named(name, q), - Err(err) => { - eprintln!("Error interpreting aggregation pipeline.\n\n{err}"); - exit(ExitCode::CouldNotReadAggregationPipeline.into()) - } - }; - - let native_query_dir = native_query_path - .parent() - .expect("parent directory of native query configuration path"); - if !(fs::try_exists(&native_query_dir).await?) { - fs::create_dir(&native_query_dir).await?; - } - - if let Err(err) = fs::write( - &native_query_path, - serde_json::to_string_pretty(&native_query)?, - ) - .await - { - eprintln!("Error writing native query configuration: {err}"); - exit(ExitCode::ErrorWriting.into()) - }; +async fn list(context: &Context) -> std::result::Result<(), anyhow::Error> { + let configuration = read_configuration(context, &[]).await?; + for (name, _) in configuration.native_queries { + println!("{}", name); + } + Ok(()) +} + +async fn create( + context: &Context, + name: String, + collection: Option, + force: bool, + pipeline_path: &Path, +) -> anyhow::Result<()> { + let native_query_path = { + let path = get_native_query_path(context, &name); + if !force && fs::try_exists(&path).await? { eprintln!( - "\nWrote native query configuration to {}", - native_query_path.to_string_lossy() + "A native query named {name} already exists at {}.", + path.to_string_lossy() ); - eprintln!(); - pretty_print_native_query_info(&mut std::io::stderr(), &native_query.value)?; - Ok(()) + eprintln!("Re-run with --force to overwrite."); + exit(ExitCode::RefusedToOverwrite.into()) } + path + }; + + let configuration = read_configuration(context, &[native_query_path.clone()]).await?; + + let pipeline = match read_pipeline(&pipeline_path).await { + Ok(p) => p, + Err(err) => { + eprintln!("Could not read aggregation pipeline.\n\n{err}"); + exit(ExitCode::CouldNotReadAggregationPipeline.into()) + } + }; + let native_query = match native_query_from_pipeline(&configuration, &name, collection, pipeline) + { + Ok(q) => WithName::named(name, q), + Err(err) => { + eprintln!("Error interpreting aggregation pipeline.\n\n{err}"); + exit(ExitCode::CouldNotReadAggregationPipeline.into()) + } + }; + + let native_query_dir = native_query_path + .parent() + .expect("parent directory of native query configuration path"); + if !(fs::try_exists(&native_query_dir).await?) { + fs::create_dir(&native_query_dir).await?; } + + if let Err(err) = fs::write( + &native_query_path, + serde_json::to_string_pretty(&native_query)?, + ) + .await + { + eprintln!("Error writing native query configuration: {err}"); + exit(ExitCode::ErrorWriting.into()) + }; + eprintln!( + "\nWrote native query configuration to {}", + native_query_path.to_string_lossy() + ); + eprintln!(); + pretty_print_native_query_info(&mut std::io::stderr(), &native_query.value)?; + Ok(()) +} + +/// Reads configuration, or exits with specific error code on error +async fn read_configuration( + context: &Context, + ignored_configs: &[PathBuf], +) -> anyhow::Result { + let configuration = match read_directory_with_ignored_configs(&context.path, ignored_configs) + .await + { + Ok(c) => c, + Err(err) => { + eprintln!("Could not read connector configuration - configuration must be initialized before creating native queries.\n\n{err:#}"); + exit(ExitCode::CouldNotReadConfiguration.into()) + } + }; + eprintln!( + "Read configuration from {}", + &context.path.to_string_lossy() + ); + Ok(configuration) } async fn read_pipeline(pipeline_path: &Path) -> anyhow::Result { From 43f5a89a09e0b47b2056db829836bc8c2c5732ed Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 12:03:27 -0800 Subject: [PATCH 03/11] add show subcommand --- crates/cli/src/exit_codes.rs | 2 + crates/cli/src/native_query/mod.rs | 46 ++++++++++++++++--- .../cli/src/native_query/pretty_printing.rs | 28 +++++++++++ crates/configuration/src/directory.rs | 21 +++++++-- crates/configuration/src/lib.rs | 4 +- 5 files changed, 90 insertions(+), 11 deletions(-) diff --git a/crates/cli/src/exit_codes.rs b/crates/cli/src/exit_codes.rs index f821caa..2b66566 100644 --- a/crates/cli/src/exit_codes.rs +++ b/crates/cli/src/exit_codes.rs @@ -5,6 +5,7 @@ pub enum ExitCode { CouldNotProcessAggregationPipeline, ErrorWriting, RefusedToOverwrite, + ResourceNotFound, } impl From for i32 { @@ -15,6 +16,7 @@ impl From for i32 { ExitCode::CouldNotProcessAggregationPipeline => 205, ExitCode::ErrorWriting => 204, ExitCode::RefusedToOverwrite => 203, + ExitCode::ResourceNotFound => 206, } } } diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index ea894c0..1a7d687 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -13,6 +13,7 @@ mod type_solver; #[cfg(test)] mod tests; +use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::process::exit; @@ -21,9 +22,10 @@ use configuration::schema::ObjectField; use configuration::{ native_query::NativeQueryRepresentation::Collection, serialized::NativeQuery, Configuration, }; -use configuration::{read_directory_with_ignored_configs, WithName}; +use configuration::{read_directory_with_ignored_configs, read_native_query_directory, WithName}; use mongodb_support::aggregate::Pipeline; -use ndc_models::CollectionName; +use ndc_models::{CollectionName, FunctionName}; +use pretty_printing::pretty_print_native_query; use tokio::fs; use crate::exit_codes::ExitCode; @@ -56,6 +58,10 @@ pub enum Command { /// List all configured native queries List, + + /// Print details of a native query identified by name. Use the list subcommand to see native + /// query names. + Show { native_query_name: String }, } pub async fn run(context: &Context, command: Command) -> anyhow::Result<()> { @@ -67,17 +73,31 @@ pub async fn run(context: &Context, command: Command) -> anyhow::Result<()> { pipeline_path, } => create(context, name, collection, force, &pipeline_path).await, Command::List => list(context).await, + Command::Show { native_query_name } => show(context, &native_query_name).await, } } -async fn list(context: &Context) -> std::result::Result<(), anyhow::Error> { - let configuration = read_configuration(context, &[]).await?; - for (name, _) in configuration.native_queries { +async fn list(context: &Context) -> anyhow::Result<()> { + let native_queries = read_native_queries(context).await?; + for (name, _) in native_queries { println!("{}", name); } Ok(()) } +async fn show(context: &Context, native_query_name: &str) -> anyhow::Result<()> { + let native_queries = read_native_queries(context).await?; + let native_query = match native_queries.get(native_query_name) { + Some(native_query) => native_query, + None => { + eprintln!("No native query named {native_query_name} found."); + exit(ExitCode::ResourceNotFound.into()) + } + }; + pretty_print_native_query(&mut std::io::stdout(), native_query)?; + Ok(()) +} + async fn create( context: &Context, name: String, @@ -100,7 +120,7 @@ async fn create( let configuration = read_configuration(context, &[native_query_path.clone()]).await?; - let pipeline = match read_pipeline(&pipeline_path).await { + let pipeline = match read_pipeline(pipeline_path).await { Ok(p) => p, Err(err) => { eprintln!("Could not read aggregation pipeline.\n\n{err}"); @@ -162,6 +182,20 @@ async fn read_configuration( Ok(configuration) } +/// Reads native queries skipping configuration processing, or exits with specific error code on error +async fn read_native_queries( + context: &Context, +) -> anyhow::Result> { + let native_queries = match read_native_query_directory(&context.path).await { + Ok(native_queries) => native_queries, + Err(err) => { + eprintln!("Could not read native queries.\n\n{err}"); + exit(ExitCode::CouldNotReadConfiguration.into()) + } + }; + Ok(native_queries) +} + async fn read_pipeline(pipeline_path: &Path) -> anyhow::Result { let input = fs::read(pipeline_path).await?; let pipeline = serde_json::from_slice(&input)?; diff --git a/crates/cli/src/native_query/pretty_printing.rs b/crates/cli/src/native_query/pretty_printing.rs index b57fc63..1e73dcc 100644 --- a/crates/cli/src/native_query/pretty_printing.rs +++ b/crates/cli/src/native_query/pretty_printing.rs @@ -2,6 +2,7 @@ use configuration::{schema::ObjectType, serialized::NativeQuery}; use itertools::Itertools; use pretty::{BoxAllocator, DocAllocator, DocBuilder, Pretty}; +/// Prints metadata for a native query, excluding its pipeline pub fn pretty_print_native_query_info( output: &mut impl std::io::Write, native_query: &NativeQuery, @@ -13,6 +14,33 @@ pub fn pretty_print_native_query_info( Ok(()) } +/// Prints metadata for a native query including its pipeline +pub fn pretty_print_native_query( + output: &mut impl std::io::Write, + native_query: &NativeQuery, +) -> std::io::Result<()> { + let allocator = BoxAllocator; + native_query_printer::<_, ()>(native_query, &allocator) + .1 + .render(80, output)?; + Ok(()) +} + +fn native_query_printer<'a, D, A>(nq: &'a NativeQuery, allocator: &'a D) -> DocBuilder<'a, D, A> +where + D: DocAllocator<'a, A>, + D::Doc: Clone, + A: Clone, +{ + let info = native_query_info_printer(nq, allocator); + let pipeline = section( + "pipeline", + allocator.text(serde_json::to_string_pretty(&nq.pipeline).unwrap()), + allocator, + ); + allocator.intersperse([info, pipeline], allocator.hardline()) +} + fn native_query_info_printer<'a, D, A>( nq: &'a NativeQuery, allocator: &'a D, diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index b3a2323..21a8d62 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -1,6 +1,7 @@ use anyhow::{anyhow, Context as _}; use futures::stream::TryStreamExt as _; use itertools::Itertools as _; +use ndc_models::FunctionName; use serde::{Deserialize, Serialize}; use std::{ collections::{BTreeMap, HashSet}, @@ -11,7 +12,10 @@ use tokio::{fs, io::AsyncWriteExt}; use tokio_stream::wrappers::ReadDirStream; use crate::{ - configuration::ConfigurationOptions, serialized::Schema, with_name::WithName, Configuration, + configuration::ConfigurationOptions, + serialized::{NativeQuery, Schema}, + with_name::WithName, + Configuration, }; pub const SCHEMA_DIRNAME: &str = "schema"; @@ -69,9 +73,7 @@ pub async fn read_directory_with_ignored_configs( .await? .unwrap_or_default(); - let native_queries = read_subdir_configs(&dir.join(NATIVE_QUERIES_DIRNAME), ignored_configs) - .await? - .unwrap_or_default(); + let native_queries = read_native_query_directory(dir).await?; let options = parse_configuration_options_file(dir).await?; @@ -80,6 +82,17 @@ pub async fn read_directory_with_ignored_configs( Configuration::validate(schema, native_mutations, native_queries, options) } +/// Read native queries only, and skip configuration processing +pub async fn read_native_query_directory( + configuration_dir: impl AsRef + Send, +) -> anyhow::Result> { + let dir = configuration_dir.as_ref(); + let native_queries = read_subdir_configs(&dir.join(NATIVE_QUERIES_DIRNAME), &[]) + .await? + .unwrap_or_default(); + Ok(native_queries) +} + /// Parse all files in a directory with one of the allowed configuration extensions according to /// the given type argument. For example if `T` is `NativeMutation` this function assumes that all /// json and yaml files in the given directory should be parsed as native mutation configurations. diff --git a/crates/configuration/src/lib.rs b/crates/configuration/src/lib.rs index c252fcc..798f232 100644 --- a/crates/configuration/src/lib.rs +++ b/crates/configuration/src/lib.rs @@ -12,7 +12,9 @@ pub use crate::directory::get_config_file_changed; pub use crate::directory::list_existing_schemas; pub use crate::directory::parse_configuration_options_file; pub use crate::directory::write_schema_directory; -pub use crate::directory::{read_directory, read_directory_with_ignored_configs}; +pub use crate::directory::{ + read_directory, read_directory_with_ignored_configs, read_native_query_directory, +}; pub use crate::directory::{ CONFIGURATION_OPTIONS_BASENAME, CONFIGURATION_OPTIONS_METADATA, NATIVE_MUTATIONS_DIRNAME, NATIVE_QUERIES_DIRNAME, SCHEMA_DIRNAME, From f5cdd1f09a593a5eca4a9e30ce107b7a577bb088 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 12:28:11 -0800 Subject: [PATCH 04/11] add delete subcommand --- crates/cli/src/native_query/mod.rs | 44 +++++++++++++++++++++------ crates/configuration/src/directory.rs | 37 ++++++++++++++++++---- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 1a7d687..8abfd32 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -56,6 +56,10 @@ pub enum Command { pipeline_path: PathBuf, }, + /// Delete a native query identified by name. Use the list subcommand to see native query + /// names. + Delete { native_query_name: String }, + /// List all configured native queries List, @@ -72,6 +76,7 @@ pub async fn run(context: &Context, command: Command) -> anyhow::Result<()> { force, pipeline_path, } => create(context, name, collection, force, &pipeline_path).await, + Command::Delete { native_query_name } => delete(context, &native_query_name).await, Command::List => list(context).await, Command::Show { native_query_name } => show(context, &native_query_name).await, } @@ -85,16 +90,20 @@ async fn list(context: &Context) -> anyhow::Result<()> { Ok(()) } +async fn delete(context: &Context, native_query_name: &str) -> anyhow::Result<()> { + let (_, path) = find_native_query(context, native_query_name).await?; + fs::remove_file(&path).await?; + eprintln!( + "Deleted native query configuration at {}", + path.to_string_lossy() + ); + Ok(()) +} + async fn show(context: &Context, native_query_name: &str) -> anyhow::Result<()> { - let native_queries = read_native_queries(context).await?; - let native_query = match native_queries.get(native_query_name) { - Some(native_query) => native_query, - None => { - eprintln!("No native query named {native_query_name} found."); - exit(ExitCode::ResourceNotFound.into()) - } - }; - pretty_print_native_query(&mut std::io::stdout(), native_query)?; + let (native_query, path) = find_native_query(context, native_query_name).await?; + println!("configuration source: {}", path.to_string_lossy()); + pretty_print_native_query(&mut std::io::stdout(), &native_query)?; Ok(()) } @@ -185,7 +194,7 @@ async fn read_configuration( /// Reads native queries skipping configuration processing, or exits with specific error code on error async fn read_native_queries( context: &Context, -) -> anyhow::Result> { +) -> anyhow::Result> { let native_queries = match read_native_query_directory(&context.path).await { Ok(native_queries) => native_queries, Err(err) => { @@ -196,6 +205,21 @@ async fn read_native_queries( Ok(native_queries) } +async fn find_native_query( + context: &Context, + name: &str, +) -> anyhow::Result<(NativeQuery, PathBuf)> { + let mut native_queries = read_native_queries(context).await?; + let (_, definition_and_path) = match native_queries.remove_entry(name) { + Some(native_query) => native_query, + None => { + eprintln!("No native query named {name} found."); + exit(ExitCode::ResourceNotFound.into()) + } + }; + Ok(definition_and_path) +} + async fn read_pipeline(pipeline_path: &Path) -> anyhow::Result { let input = fs::read(pipeline_path).await?; let pipeline = serde_json::from_slice(&input)?; diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index 21a8d62..f0be690 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -73,7 +73,11 @@ pub async fn read_directory_with_ignored_configs( .await? .unwrap_or_default(); - let native_queries = read_native_query_directory(dir).await?; + let native_queries = read_native_query_directory(dir) + .await? + .into_iter() + .map(|(name, (config, _))| (name, config)) + .collect(); let options = parse_configuration_options_file(dir).await?; @@ -85,9 +89,9 @@ pub async fn read_directory_with_ignored_configs( /// Read native queries only, and skip configuration processing pub async fn read_native_query_directory( configuration_dir: impl AsRef + Send, -) -> anyhow::Result> { +) -> anyhow::Result> { let dir = configuration_dir.as_ref(); - let native_queries = read_subdir_configs(&dir.join(NATIVE_QUERIES_DIRNAME), &[]) + let native_queries = read_subdir_configs_with_paths(&dir.join(NATIVE_QUERIES_DIRNAME), &[]) .await? .unwrap_or_default(); Ok(native_queries) @@ -102,6 +106,23 @@ async fn read_subdir_configs( subdir: &Path, ignored_configs: &[PathBuf], ) -> anyhow::Result>> +where + for<'a> T: Deserialize<'a>, + for<'a> N: Ord + ToString + Deserialize<'a>, +{ + let configs_with_paths = read_subdir_configs_with_paths(subdir, ignored_configs).await?; + let configs_without_paths = configs_with_paths.map(|cs| { + cs.into_iter() + .map(|(name, (config, _))| (name, config)) + .collect() + }); + Ok(configs_without_paths) +} + +async fn read_subdir_configs_with_paths( + subdir: &Path, + ignored_configs: &[PathBuf], +) -> anyhow::Result>> where for<'a> T: Deserialize<'a>, for<'a> N: Ord + ToString + Deserialize<'a>, @@ -111,8 +132,8 @@ where } let dir_stream = ReadDirStream::new(fs::read_dir(subdir).await?); - let configs: Vec> = dir_stream - .map_err(|err| err.into()) + let configs: Vec> = dir_stream + .map_err(anyhow::Error::from) .try_filter_map(|dir_entry| async move { // Permits regular files and symlinks, does not filter out symlinks to directories. let is_file = !(dir_entry.file_type().await?.is_dir()); @@ -141,7 +162,11 @@ where Ok(format_option.map(|format| (path, format))) }) .and_then(|(path, format)| async move { - parse_config_file::>(path, format).await + let config = parse_config_file::>(&path, format).await?; + Ok(WithName { + name: config.name, + value: (config.value, path), + }) }) .try_collect() .await?; From 3694c4e87eaf8697eaa9d8f05f757b6389b8b869 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 12:34:09 -0800 Subject: [PATCH 05/11] use block_in_place to run blocking code from async code --- crates/cli/src/native_query/mod.rs | 4 +-- .../cli/src/native_query/pretty_printing.rs | 29 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 8abfd32..73bf80c 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -103,7 +103,7 @@ async fn delete(context: &Context, native_query_name: &str) -> anyhow::Result<() async fn show(context: &Context, native_query_name: &str) -> anyhow::Result<()> { let (native_query, path) = find_native_query(context, native_query_name).await?; println!("configuration source: {}", path.to_string_lossy()); - pretty_print_native_query(&mut std::io::stdout(), &native_query)?; + pretty_print_native_query(&mut std::io::stdout(), &native_query).await?; Ok(()) } @@ -166,7 +166,7 @@ async fn create( native_query_path.to_string_lossy() ); eprintln!(); - pretty_print_native_query_info(&mut std::io::stderr(), &native_query.value)?; + pretty_print_native_query_info(&mut std::io::stderr(), &native_query.value).await?; Ok(()) } diff --git a/crates/cli/src/native_query/pretty_printing.rs b/crates/cli/src/native_query/pretty_printing.rs index 1e73dcc..440427d 100644 --- a/crates/cli/src/native_query/pretty_printing.rs +++ b/crates/cli/src/native_query/pretty_printing.rs @@ -1,29 +1,34 @@ use configuration::{schema::ObjectType, serialized::NativeQuery}; use itertools::Itertools; use pretty::{BoxAllocator, DocAllocator, DocBuilder, Pretty}; +use tokio::task; /// Prints metadata for a native query, excluding its pipeline -pub fn pretty_print_native_query_info( +pub async fn pretty_print_native_query_info( output: &mut impl std::io::Write, native_query: &NativeQuery, ) -> std::io::Result<()> { - let allocator = BoxAllocator; - native_query_info_printer::<_, ()>(native_query, &allocator) - .1 - .render(80, output)?; - Ok(()) + task::block_in_place(move || { + let allocator = BoxAllocator; + native_query_info_printer::<_, ()>(native_query, &allocator) + .1 + .render(80, output)?; + Ok(()) + }) } /// Prints metadata for a native query including its pipeline -pub fn pretty_print_native_query( +pub async fn pretty_print_native_query( output: &mut impl std::io::Write, native_query: &NativeQuery, ) -> std::io::Result<()> { - let allocator = BoxAllocator; - native_query_printer::<_, ()>(native_query, &allocator) - .1 - .render(80, output)?; - Ok(()) + task::block_in_place(move || { + let allocator = BoxAllocator; + native_query_printer::<_, ()>(native_query, &allocator) + .1 + .render(80, output)?; + Ok(()) + }) } fn native_query_printer<'a, D, A>(nq: &'a NativeQuery, allocator: &'a D) -> DocBuilder<'a, D, A> From b59eca4260743d984cb5130794d6bb5cb1e73fee Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 12:39:05 -0800 Subject: [PATCH 06/11] fix regression ignoring existing config when replacing --- crates/cli/src/native_query/mod.rs | 2 +- crates/configuration/src/directory.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 73bf80c..cf45f0f 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -195,7 +195,7 @@ async fn read_configuration( async fn read_native_queries( context: &Context, ) -> anyhow::Result> { - let native_queries = match read_native_query_directory(&context.path).await { + let native_queries = match read_native_query_directory(&context.path, &[]).await { Ok(native_queries) => native_queries, Err(err) => { eprintln!("Could not read native queries.\n\n{err}"); diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index f0be690..262d5f6 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -73,7 +73,7 @@ pub async fn read_directory_with_ignored_configs( .await? .unwrap_or_default(); - let native_queries = read_native_query_directory(dir) + let native_queries = read_native_query_directory(dir, ignored_configs) .await? .into_iter() .map(|(name, (config, _))| (name, config)) @@ -89,11 +89,13 @@ pub async fn read_directory_with_ignored_configs( /// Read native queries only, and skip configuration processing pub async fn read_native_query_directory( configuration_dir: impl AsRef + Send, + ignored_configs: &[PathBuf], ) -> anyhow::Result> { let dir = configuration_dir.as_ref(); - let native_queries = read_subdir_configs_with_paths(&dir.join(NATIVE_QUERIES_DIRNAME), &[]) - .await? - .unwrap_or_default(); + let native_queries = + read_subdir_configs_with_paths(&dir.join(NATIVE_QUERIES_DIRNAME), ignored_configs) + .await? + .unwrap_or_default(); Ok(native_queries) } From 8c728276befb0c8894aa073e14e8eb3ec953748f Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 12:54:09 -0800 Subject: [PATCH 07/11] infer native query name from input file name stem --- crates/cli/src/exit_codes.rs | 4 +++- crates/cli/src/native_query/mod.rs | 23 ++++++++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/crates/cli/src/exit_codes.rs b/crates/cli/src/exit_codes.rs index 2b66566..a8d7c24 100644 --- a/crates/cli/src/exit_codes.rs +++ b/crates/cli/src/exit_codes.rs @@ -4,6 +4,7 @@ pub enum ExitCode { CouldNotReadConfiguration, CouldNotProcessAggregationPipeline, ErrorWriting, + InvalidArguments, RefusedToOverwrite, ResourceNotFound, } @@ -15,8 +16,9 @@ impl From for i32 { ExitCode::CouldNotReadConfiguration => 202, ExitCode::CouldNotProcessAggregationPipeline => 205, ExitCode::ErrorWriting => 204, + ExitCode::InvalidArguments => 400, ExitCode::RefusedToOverwrite => 203, - ExitCode::ResourceNotFound => 206, + ExitCode::ResourceNotFound => 404, } } } diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index cf45f0f..8f04d2f 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -40,9 +40,9 @@ use self::pretty_printing::pretty_print_native_query_info; pub enum Command { /// Create a native query from a JSON file containing an aggregation pipeline Create { - /// Name that will identify the query in your data graph - #[arg(long, short = 'n', required = true)] - name: String, + /// Name that will identify the query in your data graph (defaults to base name of pipeline file) + #[arg(long, short = 'n')] + name: Option, /// Name of the collection that acts as input for the pipeline - omit for a pipeline that does not require input #[arg(long, short = 'c')] @@ -52,7 +52,8 @@ pub enum Command { #[arg(long, short = 'f')] force: bool, - /// Path to a JSON file with an aggregation pipeline + /// Path to a JSON file with an aggregation pipeline that specifies your custom query. This + /// is a value that could be given to the MongoDB command db..aggregate(). pipeline_path: PathBuf, }, @@ -109,11 +110,23 @@ async fn show(context: &Context, native_query_name: &str) -> anyhow::Result<()> async fn create( context: &Context, - name: String, + name: Option, collection: Option, force: bool, pipeline_path: &Path, ) -> anyhow::Result<()> { + let name = match name.or_else(|| { + pipeline_path + .file_stem() + .map(|os_str| os_str.to_string_lossy().to_string()) + }) { + Some(name) => name, + None => { + eprintln!("Could not determine name for native query."); + exit(ExitCode::InvalidArguments.into()) + } + }; + let native_query_path = { let path = get_native_query_path(context, &name); if !force && fs::try_exists(&path).await? { From 8eb6fe641b059d952042666482d7c79aa7aa2ada Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 14:05:52 -0800 Subject: [PATCH 08/11] add colors to output --- Cargo.lock | 1 + crates/cli/Cargo.toml | 2 +- crates/cli/src/native_query/mod.rs | 15 +- .../cli/src/native_query/pretty_printing.rs | 146 +++++++++++++----- 4 files changed, 124 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b003a9a..786ae48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2359,6 +2359,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b55c4d17d994b637e2f4daf6e5dc5d660d209d5642377d675d7a1c3ab69fa579" dependencies = [ "arrayvec", + "termcolor", "typed-arena", "unicode-width", ] diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 169b6b6..c19d686 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -21,7 +21,7 @@ itertools = { workspace = true } ndc-models = { workspace = true } nom = { version = "^7.1.3", optional = true } nonempty = "^0.10.0" -pretty = { version = "^0.12.3", optional = true } +pretty = { version = "^0.12.3", features = ["termcolor"], optional = true } ref-cast = { workspace = true } regex = "^1.11.1" serde = { workspace = true } diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 8f04d2f..99edc7c 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -25,6 +25,7 @@ use configuration::{ use configuration::{read_directory_with_ignored_configs, read_native_query_directory, WithName}; use mongodb_support::aggregate::Pipeline; use ndc_models::{CollectionName, FunctionName}; +use pretty::termcolor::{ColorChoice, StandardStream}; use pretty_printing::pretty_print_native_query; use tokio::fs; @@ -103,8 +104,12 @@ async fn delete(context: &Context, native_query_name: &str) -> anyhow::Result<() async fn show(context: &Context, native_query_name: &str) -> anyhow::Result<()> { let (native_query, path) = find_native_query(context, native_query_name).await?; - println!("configuration source: {}", path.to_string_lossy()); - pretty_print_native_query(&mut std::io::stdout(), &native_query).await?; + pretty_print_native_query( + &mut StandardStream::stdout(ColorChoice::Auto), + &native_query, + &path, + ) + .await?; Ok(()) } @@ -179,7 +184,11 @@ async fn create( native_query_path.to_string_lossy() ); eprintln!(); - pretty_print_native_query_info(&mut std::io::stderr(), &native_query.value).await?; + pretty_print_native_query_info( + &mut StandardStream::stdout(ColorChoice::Auto), + &native_query.value, + ) + .await?; Ok(()) } diff --git a/crates/cli/src/native_query/pretty_printing.rs b/crates/cli/src/native_query/pretty_printing.rs index 440427d..7543393 100644 --- a/crates/cli/src/native_query/pretty_printing.rs +++ b/crates/cli/src/native_query/pretty_printing.rs @@ -1,77 +1,98 @@ +use std::path::Path; + use configuration::{schema::ObjectType, serialized::NativeQuery}; use itertools::Itertools; -use pretty::{BoxAllocator, DocAllocator, DocBuilder, Pretty}; +use pretty::{ + termcolor::{Color, ColorSpec, StandardStream}, + BoxAllocator, DocAllocator, DocBuilder, Pretty, +}; use tokio::task; /// Prints metadata for a native query, excluding its pipeline pub async fn pretty_print_native_query_info( - output: &mut impl std::io::Write, + output: &mut StandardStream, native_query: &NativeQuery, ) -> std::io::Result<()> { task::block_in_place(move || { let allocator = BoxAllocator; - native_query_info_printer::<_, ()>(native_query, &allocator) + native_query_info_printer(native_query, &allocator) .1 - .render(80, output)?; + .render_colored(80, output)?; Ok(()) }) } /// Prints metadata for a native query including its pipeline pub async fn pretty_print_native_query( - output: &mut impl std::io::Write, + output: &mut StandardStream, native_query: &NativeQuery, + path: &Path, ) -> std::io::Result<()> { task::block_in_place(move || { let allocator = BoxAllocator; - native_query_printer::<_, ()>(native_query, &allocator) + native_query_printer(native_query, path, &allocator) .1 - .render(80, output)?; + .render_colored(80, output)?; Ok(()) }) } -fn native_query_printer<'a, D, A>(nq: &'a NativeQuery, allocator: &'a D) -> DocBuilder<'a, D, A> +fn native_query_printer<'a, D>( + nq: &'a NativeQuery, + path: &'a Path, + allocator: &'a D, +) -> DocBuilder<'a, D, ColorSpec> where - D: DocAllocator<'a, A>, + D: DocAllocator<'a, ColorSpec>, D::Doc: Clone, - A: Clone, { + let source = definition_list_entry( + "configuration source", + allocator.text(path.to_string_lossy()), + allocator, + ); let info = native_query_info_printer(nq, allocator); let pipeline = section( "pipeline", allocator.text(serde_json::to_string_pretty(&nq.pipeline).unwrap()), allocator, ); - allocator.intersperse([info, pipeline], allocator.hardline()) + allocator.intersperse([source, info, pipeline], allocator.hardline()) } -fn native_query_info_printer<'a, D, A>( +fn native_query_info_printer<'a, D>( nq: &'a NativeQuery, allocator: &'a D, -) -> DocBuilder<'a, D, A> +) -> DocBuilder<'a, D, ColorSpec> where - D: DocAllocator<'a, A>, + D: DocAllocator<'a, ColorSpec>, D::Doc: Clone, - A: Clone, { let input_collection = nq.input_collection.as_ref().map(|collection| { - allocator - .text("input collection: ") - .append(allocator.text(collection.to_string())) + definition_list_entry( + "input collection", + allocator.text(collection.to_string()), + allocator, + ) }); - let representation = Some( - allocator - .text("representation: ") - .append(allocator.text(nq.representation.to_str())), - ); + let representation = Some(definition_list_entry( + "representation", + allocator.text(nq.representation.to_str()), + allocator, + )); let parameters = if !nq.arguments.is_empty() { let params = nq.arguments.iter().map(|(name, definition)| { allocator - .text(format!("{name}: ")) - .append(allocator.text(format!("{}", definition.r#type))) + .text(name.to_string()) + .annotate(field_name()) + .append(allocator.text(": ")) + .append( + allocator + .text(definition.r#type.to_string()) + .annotate(type_expression()), + ) }); Some(section( "parameters", @@ -100,6 +121,7 @@ where let docs = other_object_types.into_iter().map(|(name, definition)| { allocator .text(format!("{name} ")) + .annotate(object_type_name()) .append(object_type_printer(definition, allocator)) }); let separator = allocator.line().append(allocator.line()); @@ -126,16 +148,21 @@ where ) } -fn object_type_printer<'a, D, A>(ot: &'a ObjectType, allocator: &'a D) -> DocBuilder<'a, D, A> +fn object_type_printer<'a, D>(ot: &'a ObjectType, allocator: &'a D) -> DocBuilder<'a, D, ColorSpec> where - D: DocAllocator<'a, A>, + D: DocAllocator<'a, ColorSpec>, D::Doc: Clone, - A: Clone, { let fields = ot.fields.iter().map(|(name, definition)| { allocator - .text(format!("{name}: ")) - .append(allocator.text(format!("{}", definition.r#type))) + .text(name.to_string()) + .annotate(field_name()) + .append(allocator.text(": ")) + .append( + allocator + .text(definition.r#type.to_string()) + .annotate(type_expression()), + ) }); let separator = allocator.text(",").append(allocator.line()); let body = allocator.intersperse(fields, separator); @@ -145,17 +172,35 @@ where ) } -fn section<'a, D, A>( +fn definition_list_entry<'a, D>( + label: &'a str, + body: impl Pretty<'a, D, ColorSpec>, + allocator: &'a D, +) -> DocBuilder<'a, D, ColorSpec> +where + D: DocAllocator<'a, ColorSpec>, + D::Doc: Clone, +{ + allocator + .text(label) + .annotate(definition_list_label()) + .append(allocator.text(": ")) + .append(body) +} + +fn section<'a, D>( heading: &'a str, - body: impl Pretty<'a, D, A>, + body: impl Pretty<'a, D, ColorSpec>, allocator: &'a D, -) -> DocBuilder<'a, D, A> +) -> DocBuilder<'a, D, ColorSpec> where - D: DocAllocator<'a, A>, + D: DocAllocator<'a, ColorSpec>, D::Doc: Clone, - A: Clone, { - let heading_doc = allocator.text("## ").append(heading); + let heading_doc = allocator + .text("## ") + .append(heading) + .annotate(section_heading()); allocator .line() .append(heading_doc) @@ -163,3 +208,32 @@ where .append(allocator.line()) .append(body) } + +fn section_heading() -> ColorSpec { + let mut color = ColorSpec::new(); + color.set_fg(Some(Color::Red)); + color.set_bold(true); + color +} + +fn definition_list_label() -> ColorSpec { + let mut color = ColorSpec::new(); + color.set_fg(Some(Color::Blue)); + color +} + +fn field_name() -> ColorSpec { + let mut color = ColorSpec::new(); + color.set_fg(Some(Color::Yellow)); + color +} + +fn object_type_name() -> ColorSpec { + // placeholder in case we want styling here in the future + ColorSpec::new() +} + +fn type_expression() -> ColorSpec { + // placeholder in case we want styling here in the future + ColorSpec::new() +} From 147701ac055465f02062e38ede2907812fc10a98 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Fri, 22 Nov 2024 14:13:19 -0800 Subject: [PATCH 09/11] add option to suppress color --- crates/cli/src/lib.rs | 1 + crates/cli/src/main.rs | 5 +++++ crates/cli/src/native_query/mod.rs | 21 ++++++++++----------- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index e09ae64..3fb92b9 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -43,6 +43,7 @@ pub enum Command { pub struct Context { pub path: PathBuf, pub connection_uri: Option, + pub display_color: bool, } /// Run a command in a given directory. diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 20b508b..c358be9 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -31,6 +31,10 @@ pub struct Args { )] pub connection_uri: Option, + /// Disable color in command output. + #[arg(long = "no-color", short = 'C')] + pub no_color: bool, + /// The command to invoke. #[command(subcommand)] pub subcommand: Command, @@ -49,6 +53,7 @@ pub async fn main() -> anyhow::Result<()> { let context = Context { path, connection_uri: args.connection_uri, + display_color: !args.no_color, }; run(args.subcommand, &context).await?; Ok(()) diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 99edc7c..64acf9d 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -104,12 +104,7 @@ async fn delete(context: &Context, native_query_name: &str) -> anyhow::Result<() async fn show(context: &Context, native_query_name: &str) -> anyhow::Result<()> { let (native_query, path) = find_native_query(context, native_query_name).await?; - pretty_print_native_query( - &mut StandardStream::stdout(ColorChoice::Auto), - &native_query, - &path, - ) - .await?; + pretty_print_native_query(&mut stdout(context), &native_query, &path).await?; Ok(()) } @@ -184,11 +179,7 @@ async fn create( native_query_path.to_string_lossy() ); eprintln!(); - pretty_print_native_query_info( - &mut StandardStream::stdout(ColorChoice::Auto), - &native_query.value, - ) - .await?; + pretty_print_native_query_info(&mut stdout(context), &native_query.value).await?; Ok(()) } @@ -293,3 +284,11 @@ pub fn native_query_from_pipeline( description: None, }) } + +fn stdout(context: &Context) -> StandardStream { + if context.display_color { + StandardStream::stdout(ColorChoice::Auto) + } else { + StandardStream::stdout(ColorChoice::Never) + } +} From bbbd7eaec66ce3c32b1146f9c265cc6f00d0d6e1 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Sun, 24 Nov 2024 17:00:24 -0800 Subject: [PATCH 10/11] better error message when parsing a stage fails --- crates/cli/src/native_query/error.rs | 4 +-- crates/cli/src/native_query/mod.rs | 2 +- crates/cli/src/native_query/pipeline/mod.rs | 34 ++++++++++++--------- crates/cli/src/native_query/tests.rs | 10 ++---- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/crates/cli/src/native_query/error.rs b/crates/cli/src/native_query/error.rs index 3013931..6202168 100644 --- a/crates/cli/src/native_query/error.rs +++ b/crates/cli/src/native_query/error.rs @@ -87,10 +87,10 @@ pub enum Error { #[error("Type inference is not currently implemented for the aggregation expression operator, {0}. Please file a bug report, and declare types for your native query by hand for the time being.")] UnknownAggregationOperator(String), - #[error("Type inference is not currently implemented for {stage}, stage number {} in your aggregation pipeline. Please file a bug report, and declare types for your native query by hand for the time being.", stage_index + 1)] + #[error("Type inference is not currently implemented for{} stage number {} in your aggregation pipeline. Please file a bug report, and declare types for your native query by hand for the time being.", match stage_name { Some(name) => format!(" {name},"), None => "".to_string() }, stage_index + 1)] UnknownAggregationStage { stage_index: usize, - stage: bson::Document, + stage_name: Option<&'static str>, }, #[error("Native query input collection, \"{0}\", is not defined in the connector schema")] diff --git a/crates/cli/src/native_query/mod.rs b/crates/cli/src/native_query/mod.rs index 64acf9d..b5e6837 100644 --- a/crates/cli/src/native_query/mod.rs +++ b/crates/cli/src/native_query/mod.rs @@ -153,7 +153,7 @@ async fn create( { Ok(q) => WithName::named(name, q), Err(err) => { - eprintln!("Error interpreting aggregation pipeline.\n\n{err}"); + eprintln!("Error interpreting aggregation pipeline. If you are not able to resolve this error you can add the native query by writing the configuration file directly in {}.\n\n{err}", native_query_path.to_string_lossy()); exit(ExitCode::CouldNotReadAggregationPipeline.into()) } }; diff --git a/crates/cli/src/native_query/pipeline/mod.rs b/crates/cli/src/native_query/pipeline/mod.rs index 664670e..acc8004 100644 --- a/crates/cli/src/native_query/pipeline/mod.rs +++ b/crates/cli/src/native_query/pipeline/mod.rs @@ -69,7 +69,10 @@ fn infer_stage_output_type( stage: &Stage, ) -> Result> { let output_type = match stage { - Stage::AddFields(_) => todo!("add fields stage"), + Stage::AddFields(_) => Err(Error::UnknownAggregationStage { + stage_index, + stage_name: Some("$addFields"), + })?, Stage::Documents(docs) => { let doc_constraints = docs .iter() @@ -112,7 +115,10 @@ fn infer_stage_output_type( )?; None } - Stage::Lookup { .. } => todo!("lookup stage"), + Stage::Lookup { .. } => Err(Error::UnknownAggregationStage { + stage_index, + stage_name: Some("$lookup"), + })?, Stage::Group { key_expression, accumulators, @@ -125,8 +131,14 @@ fn infer_stage_output_type( )?; Some(TypeConstraint::Object(object_type_name)) } - Stage::Facet(_) => todo!("facet stage"), - Stage::Count(_) => todo!("count stage"), + Stage::Facet(_) => Err(Error::UnknownAggregationStage { + stage_index, + stage_name: Some("$facet"), + })?, + Stage::Count(_) => Err(Error::UnknownAggregationStage { + stage_index, + stage_name: Some("$count"), + })?, Stage::Project(doc) => { let augmented_type = project_stage::infer_type_from_project_stage( context, @@ -160,16 +172,10 @@ fn infer_stage_output_type( include_array_index.as_deref(), *preserve_null_and_empty_arrays, )?), - Stage::Other(doc) => { - context.add_warning(Error::UnknownAggregationStage { - stage_index, - stage: doc.clone(), - }); - // We don't know what the type is here so we represent it with an unconstrained type - // variable. - let type_variable = context.new_type_variable(Variance::Covariant, []); - Some(TypeConstraint::Variable(type_variable)) - } + Stage::Other(_) => Err(Error::UnknownAggregationStage { + stage_index, + stage_name: None, + })?, }; Ok(output_type) } diff --git a/crates/cli/src/native_query/tests.rs b/crates/cli/src/native_query/tests.rs index 3e69204..1a54372 100644 --- a/crates/cli/src/native_query/tests.rs +++ b/crates/cli/src/native_query/tests.rs @@ -3,10 +3,8 @@ use std::collections::BTreeMap; use anyhow::Result; use configuration::{ native_query::NativeQueryRepresentation::Collection, - read_directory, schema::{ObjectField, ObjectType, Type}, serialized::NativeQuery, - Configuration, }; use googletest::prelude::*; use itertools::Itertools as _; @@ -23,7 +21,7 @@ use super::native_query_from_pipeline; #[tokio::test] async fn infers_native_query_from_pipeline() -> Result<()> { - let config = read_configuration().await?; + let config = mflix_config(); let pipeline = Pipeline::new(vec![Stage::Documents(vec![ doc! { "foo": 1 }, doc! { "bar": 2 }, @@ -78,7 +76,7 @@ async fn infers_native_query_from_pipeline() -> Result<()> { #[tokio::test] async fn infers_native_query_from_non_trivial_pipeline() -> Result<()> { - let config = read_configuration().await?; + let config = mflix_config(); let pipeline = Pipeline::new(vec![ Stage::ReplaceWith(Selection::new(doc! { "title": "$title", @@ -508,7 +506,3 @@ where }) .collect() } - -async fn read_configuration() -> Result { - read_directory("../../fixtures/hasura/sample_mflix/connector").await -} From 49eca309b257c20cadb17cffac99f1745e2f8699 Mon Sep 17 00:00:00 2001 From: Jesse Hallett Date: Sun, 24 Nov 2024 17:06:14 -0800 Subject: [PATCH 11/11] fix critical typo in stage tag name --- crates/mongodb-support/src/aggregate/stage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/mongodb-support/src/aggregate/stage.rs b/crates/mongodb-support/src/aggregate/stage.rs index 76ee4e9..635e2c2 100644 --- a/crates/mongodb-support/src/aggregate/stage.rs +++ b/crates/mongodb-support/src/aggregate/stage.rs @@ -168,7 +168,7 @@ pub enum Stage { /// $replaceWith is an alias for $replaceRoot stage. /// /// See https://www.mongodb.com/docs/manual/reference/operator/aggregation/replaceRoot/#mongodb-pipeline-pipe.-replaceRoot - #[serde(rename = "$replaceWith", rename_all = "camelCase")] + #[serde(rename = "$replaceRoot", rename_all = "camelCase")] ReplaceRoot { new_root: Selection }, /// Replaces a document with the specified embedded document. The operation replaces all