diff --git a/CHANGELOG.md b/CHANGELOG.md index ea464175..4dab7492 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ This changelog documents the changes between release versions. ## [Unreleased] +### Fixed + +- Escape field names or aliases with invalid characters in field selections ([#175](https://github.com/hasura/ndc-mongodb/pull/175)) + ## [1.8.3] - 2025-07-08 ### Fixed diff --git a/crates/integration-tests/src/tests/mod.rs b/crates/integration-tests/src/tests/mod.rs index 1956d231..d1809e83 100644 --- a/crates/integration-tests/src/tests/mod.rs +++ b/crates/integration-tests/src/tests/mod.rs @@ -16,4 +16,5 @@ mod native_mutation; mod native_query; mod permissions; mod remote_relationship; +mod selection; mod sorting; diff --git a/crates/integration-tests/src/tests/selection.rs b/crates/integration-tests/src/tests/selection.rs new file mode 100644 index 00000000..9f530322 --- /dev/null +++ b/crates/integration-tests/src/tests/selection.rs @@ -0,0 +1,20 @@ +use crate::{connector::Connector, run_connector_query}; +use insta::assert_yaml_snapshot; +use ndc_test_helpers::{asc, field, query, query_request}; + +#[tokio::test] +async fn selects_fields_with_weird_aliases() -> anyhow::Result<()> { + assert_yaml_snapshot!( + run_connector_query( + Connector::SampleMflix, + query_request().collection("movies").query( + query() + .fields([field!("foo.bar" => "title"), field!("year")]) + .limit(10) + .order_by([asc!("_id")]), + ) + ) + .await? + ); + Ok(()) +} diff --git a/crates/integration-tests/src/tests/snapshots/integration_tests__tests__selection__selects_fields_with_weird_aliases.snap b/crates/integration-tests/src/tests/snapshots/integration_tests__tests__selection__selects_fields_with_weird_aliases.snap new file mode 100644 index 00000000..e5d864bf --- /dev/null +++ b/crates/integration-tests/src/tests/snapshots/integration_tests__tests__selection__selects_fields_with_weird_aliases.snap @@ -0,0 +1,25 @@ +--- +source: crates/integration-tests/src/tests/selection.rs +expression: "run_connector_query(Connector::SampleMflix,\nquery_request().collection(\"movies\").query(query().fields([field!(\"foo.bar\" =>\n\"title\"), field!(\"year\")]).limit(10).order_by([asc!(\"_id\")]),)).await?" +--- +- rows: + - foo.bar: Blacksmith Scene + year: 1893 + - foo.bar: The Great Train Robbery + year: 1903 + - foo.bar: The Land Beyond the Sunset + year: 1912 + - foo.bar: A Corner in Wheat + year: 1909 + - foo.bar: "Winsor McCay, the Famous Cartoonist of the N.Y. Herald and His Moving Comics" + year: 1911 + - foo.bar: Traffic in Souls + year: 1913 + - foo.bar: Gertie the Dinosaur + year: 1914 + - foo.bar: In the Land of the Head Hunters + year: 1914 + - foo.bar: The Perils of Pauline + year: 1914 + - foo.bar: The Birth of a Nation + year: 1915 diff --git a/crates/mongodb-agent-common/src/mongodb/selection.rs b/crates/mongodb-agent-common/src/mongodb/selection.rs index 614594c1..f60ef7dc 100644 --- a/crates/mongodb-agent-common/src/mongodb/selection.rs +++ b/crates/mongodb-agent-common/src/mongodb/selection.rs @@ -1,19 +1,18 @@ use indexmap::IndexMap; -use mongodb::bson::{doc, Bson, Document}; +use mongodb::bson::{bson, doc, Bson, Document}; use mongodb_support::aggregate::Selection; use ndc_models::FieldName; use crate::{ interface_types::MongoAgentError, mongo_query_plan::{Field, NestedArray, NestedField, NestedObject, QueryPlan}, - mongodb::sanitize::get_field, + mongodb::sanitize::{get_field, is_name_safe}, query::column_ref::ColumnRef, }; pub fn selection_from_query_request( query_request: &QueryPlan, ) -> Result { - // let fields = (&query_request.query.fields).flatten().unwrap_or_default(); let empty_map = IndexMap::new(); let fields = if let Some(fs) = &query_request.query.fields { fs @@ -28,10 +27,11 @@ fn from_query_request_helper( parent: Option>, field_selection: &IndexMap, ) -> Result { - field_selection + let selection_doc = field_selection .iter() .map(|(key, value)| Ok((key.to_string(), selection_for_field(parent.clone(), value)?))) - .collect() + .collect::>()?; + Ok(escape_invalid_keys(selection_doc)) } /// Wraps column reference with an `$isNull` check. That catches cases where a field is missing @@ -175,6 +175,27 @@ fn nested_column_reference<'a>( } } +/// If any key in our selection document contains invalid characters then we have to switch syntax +/// to using $setField to escape the field name. Unfortunately this syntax is only capable of +/// adding _one_ field to an object at a time, so we have to nest invocations of $setField for +/// every selected field. +fn escape_invalid_keys(doc: Document) -> Document { + if doc.keys().all(is_name_safe) { + return doc; + } + doc.into_iter() + .fold(None, |input: Option, (key, value)| { + Some(doc! { + "$setField": { + "field": key, + "value": value, + "input": if let Some(prev_doc) = input { prev_doc.into() } else { bson!("$$CURRENT") }, + } + }) + }) + .expect("selected at least one field") // safety: if the doc was empty we would have hit the early return +} + #[cfg(test)] mod tests { use configuration::Configuration;