hasura · hallettj · Jun 12, 2024 · Jun 5, 2024 · Jun 5, 2024 · Jun 6, 2024
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,9 @@ This changelog documents the changes between release versions.
 
 ## [Unreleased]
 - Support filtering and sorting by fields of related collections ([#72](https://github.com/hasura/ndc-mongodb/pull/72))
+- Support for root collection column references ([#75](https://github.com/hasura/ndc-mongodb/pull/75))
+- Fix for databases with field names that begin with a dollar sign, or that contain dots ([#74](https://github.com/hasura/ndc-mongodb/pull/74))
+- Implement column-to-column comparisons within the same collection ([#74](https://github.com/hasura/ndc-mongodb/pull/74))
 
 ## [0.0.6] - 2024-05-01
 - Enables logging events from the MongoDB driver by setting the `RUST_LOG` variable ([#67](https://github.com/hasura/ndc-mongodb/pull/67))

diff --git a/crates/integration-tests/src/graphql.rs b/crates/integration-tests/src/graphql.rs
@@ -1,3 +1,5 @@
+use std::collections::BTreeMap;
+
 use reqwest::Client;
 use serde::{Deserialize, Serialize};
 use serde_json::{to_value, Value};
@@ -12,6 +14,8 @@ pub struct GraphQLRequest {
     operation_name: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     variables: Option<Value>,
+    #[serde(skip_serializing)]
+    headers: BTreeMap<String, String>,
 }
 
 impl GraphQLRequest {
@@ -20,6 +24,7 @@ impl GraphQLRequest {
             query,
             operation_name: Default::default(),
             variables: Default::default(),
+            headers: [("x-hasura-role".into(), "admin".into())].into(),
         }
     }
 
@@ -33,15 +38,25 @@ impl GraphQLRequest {
         self
     }
 
+    pub fn headers(
+        mut self,
+        headers: impl IntoIterator<Item = (impl ToString, impl ToString)>,
+    ) -> Self {
+        self.headers = headers
+            .into_iter()
+            .map(|(key, value)| (key.to_string(), value.to_string()))
+            .collect();
+        self
+    }
+
     pub async fn run(&self) -> anyhow::Result<GraphQLResponse> {
         let graphql_url = get_graphql_url()?;
         let client = Client::new();
-        let response = client
-            .post(graphql_url)
-            .header("x-hasura-role", "admin")
-            .json(self)
-            .send()
-            .await?;
+        let mut request_builder = client.post(graphql_url).json(self);
+        for (key, value) in self.headers.iter() {
+            request_builder = request_builder.header(key, value);
+        }
+        let response = request_builder.send().await?;
         let graphql_response = response.json().await?;
         Ok(graphql_response)
     }

diff --git a/crates/integration-tests/src/tests/mod.rs b/crates/integration-tests/src/tests/mod.rs
@@ -11,4 +11,5 @@ mod basic;
 mod local_relationship;
 mod native_mutation;
 mod native_query;
+mod permissions;
 mod remote_relationship;
diff --git a/crates/integration-tests/src/tests/permissions.rs b/crates/integration-tests/src/tests/permissions.rs
@@ -0,0 +1,36 @@
+use crate::graphql_query;
+use insta::assert_yaml_snapshot;
+
+#[tokio::test]
+async fn filters_results_according_to_configured_permissions() -> anyhow::Result<()> {
+    assert_yaml_snapshot!(
+        graphql_query(
+            r#"
+            query {
+              users(order_by: {id: Asc}) {
+                id
+                name
+                email
+                comments(limit: 5, order_by: {id: Asc}) {
+                  date
+                  email
+                  text
+                }
+              }
+              comments(limit: 5, order_by: {id: Asc}) {
+                date
+                email
+                text
+              }
+            }
+            "#
+        )
+        .headers([
+            ("x-hasura-role", "user"),
+            ("x-hasura-user-id", "59b99db4cfa9a34dcd7885b6"),
+        ])
+        .run()
+        .await?
+    );
+    Ok(())
+}
diff --git a/...ation_tests__tests__permissions__filters_results_according_to_configured_permissions.snap b/...ation_tests__tests__permissions__filters_results_according_to_configured_permissions.snap
@@ -0,0 +1,42 @@
+---
+source: crates/integration-tests/src/tests/permissions.rs
+expression: "graphql_query(r#\"\n            query {\n              users(limit: 5) {\n                id\n                name\n                email\n                comments(limit: 5) {\n                  date\n                  email\n                  text\n                }\n              }\n              comments(limit: 5) {\n                date\n                email\n                text\n              }\n            }\n            \"#).headers([(\"x-hasura-role\",\n                            \"user\"),\n                        (\"x-hasura-user-id\",\n                            \"59b99db4cfa9a34dcd7885b6\")]).run().await?"
+---
+data:
+  users:
+    - id: 59b99db4cfa9a34dcd7885b6
+      name: Ned Stark
+      email: sean_bean@gameofthron.es
+      comments:
+        - date: "2000-01-21T03:17:04.000000000Z"
+          email: sean_bean@gameofthron.es
+          text: Illo nostrum enim sequi doloremque dolore saepe beatae. Iusto alias odit quaerat id dolores. Dolore quaerat accusantium esse voluptatibus. Aspernatur fuga exercitationem explicabo.
+        - date: "2005-09-24T16:22:38.000000000Z"
+          email: sean_bean@gameofthron.es
+          text: Architecto eos eum iste facilis. Sunt aperiam fugit nihil quas.
+        - date: "1978-10-22T23:49:33.000000000Z"
+          email: sean_bean@gameofthron.es
+          text: Aspernatur ullam blanditiis qui dolorum. Magnam minima suscipit esse. Laudantium voluptates incidunt quia saepe.
+        - date: "2013-08-15T07:24:54.000000000Z"
+          email: sean_bean@gameofthron.es
+          text: Ullam error officiis incidunt praesentium debitis. Rerum repudiandae illum reprehenderit aut non. Iusto eum autem veniam eveniet temporibus sed. Accusamus sint sed veritatis eaque.
+        - date: "2004-12-22T12:53:43.000000000Z"
+          email: sean_bean@gameofthron.es
+          text: Ducimus sunt neque sint nesciunt quis vero. Debitis ex non asperiores voluptatem iusto possimus. Doloremque blanditiis consequuntur explicabo placeat commodi repudiandae.
+  comments:
+    - date: "2000-01-21T03:17:04.000000000Z"
+      email: sean_bean@gameofthron.es
+      text: Illo nostrum enim sequi doloremque dolore saepe beatae. Iusto alias odit quaerat id dolores. Dolore quaerat accusantium esse voluptatibus. Aspernatur fuga exercitationem explicabo.
+    - date: "2005-09-24T16:22:38.000000000Z"
+      email: sean_bean@gameofthron.es
+      text: Architecto eos eum iste facilis. Sunt aperiam fugit nihil quas.
+    - date: "1978-10-22T23:49:33.000000000Z"
+      email: sean_bean@gameofthron.es
+      text: Aspernatur ullam blanditiis qui dolorum. Magnam minima suscipit esse. Laudantium voluptates incidunt quia saepe.
+    - date: "2013-08-15T07:24:54.000000000Z"
+      email: sean_bean@gameofthron.es
+      text: Ullam error officiis incidunt praesentium debitis. Rerum repudiandae illum reprehenderit aut non. Iusto eum autem veniam eveniet temporibus sed. Accusamus sint sed veritatis eaque.
+    - date: "2004-12-22T12:53:43.000000000Z"
+      email: sean_bean@gameofthron.es
+      text: Ducimus sunt neque sint nesciunt quis vero. Debitis ex non asperiores voluptatem iusto possimus. Doloremque blanditiis consequuntur explicabo placeat commodi repudiandae.
+errors: ~
diff --git a/crates/mongodb-agent-common/src/mongodb/selection.rs b/crates/mongodb-agent-common/src/mongodb/selection.rs
@@ -323,6 +323,10 @@ mod tests {
 
         let query_plan = plan_for_query_request(&students_config(), query_request)?;
 
+        // TODO: MDB-164 This selection illustrates that we end up looking up the relationship
+        // twice (once with the key `class_students`, and then with the key `class_students_0`).
+        // This is because the queries on the two relationships have different scope names. The
+        // query would work with just one lookup. Can we do that optimization?
         let selection = Selection::from_query_request(&query_plan)?;
         assert_eq!(
             Into::<Document>::into(selection),
@@ -340,7 +344,7 @@ mod tests {
                 "students": {
                     "rows": {
                         "$map": {
-                            "input": { "$getField": { "$literal": "class_students" } },
+                            "input": { "$getField": { "$literal": "class_students_0" } },
                             "in": {
                                 "student_name": "$$this.student_name"
                             },

diff --git a/crates/mongodb-agent-common/src/query/column_ref.rs b/crates/mongodb-agent-common/src/query/column_ref.rs
@@ -1,6 +1,7 @@
 use std::{borrow::Cow, iter::once};
 
 use mongodb::bson::{doc, Bson};
+use ndc_query_plan::Scope;
 
 use crate::{mongo_query_plan::ComparisonTarget, mongodb::sanitize::is_name_safe};
 
@@ -49,13 +50,16 @@ fn from_target(column: &ComparisonTarget) -> ColumnRef<'_> {
             // one element, and we know it does because we start the iterable with `name`
             from_path(None, name_and_path).unwrap()
         }
-        ComparisonTarget::RootCollectionColumn {
-            name, field_path, ..
+        ComparisonTarget::ColumnInScope {
+            name,
+            field_path,
+            scope,
+            ..
         } => {
             // "$$ROOT" is not actually a valid match key, but cheating here makes the
             // implementation much simpler. This match branch produces a ColumnRef::Expression
             // in all cases.
-            let init = ColumnRef::MatchKey("$ROOT".into());
+            let init = ColumnRef::MatchKey(format!("${}", name_from_scope(scope)).into());
             // The None case won't come up if the input to [from_target_helper] has at least
             // one element, and we know it does because we start the iterable with `name`
             let col_ref =
@@ -69,6 +73,13 @@ fn from_target(column: &ComparisonTarget) -> ColumnRef<'_> {
     }
 }
 
+pub fn name_from_scope(scope: &Scope) -> Cow<'_, str> {
+    match scope {
+        Scope::Root => "scope_root".into(),
+        Scope::Named(name) => name.into(),
+    }
+}
+
 fn from_path<'a>(
     init: Option<ColumnRef<'a>>,
     path: impl IntoIterator<Item = &'a String>,
@@ -140,6 +151,7 @@ mod tests {
     use configuration::MongoScalarType;
     use mongodb::bson::doc;
     use mongodb_support::BsonScalarType;
+    use ndc_query_plan::Scope;
     use pretty_assertions::assert_eq;
 
     use crate::mongo_query_plan::{ComparisonTarget, Type};
@@ -255,29 +267,31 @@ mod tests {
 
     #[test]
     fn produces_dot_separated_root_column_reference() -> anyhow::Result<()> {
-        let target = ComparisonTarget::RootCollectionColumn {
+        let target = ComparisonTarget::ColumnInScope {
             name: "field".into(),
             field_path: Some(vec!["prop1".into(), "prop2".into()]),
             column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
+            scope: Scope::Root,
         };
         let actual = ColumnRef::from_comparison_target(&target);
-        let expected = ColumnRef::Expression("$$ROOT.field.prop1.prop2".into());
+        let expected = ColumnRef::Expression("$$scope_root.field.prop1.prop2".into());
         assert_eq!(actual, expected);
         Ok(())
     }
 
     #[test]
     fn escapes_unsafe_field_name_in_root_column_reference() -> anyhow::Result<()> {
-        let target = ComparisonTarget::RootCollectionColumn {
+        let target = ComparisonTarget::ColumnInScope {
             name: "$field".into(),
             field_path: Default::default(),
             column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
+            scope: Scope::Named("scope_0".into()),
         };
         let actual = ColumnRef::from_comparison_target(&target);
         let expected = ColumnRef::Expression(
             doc! {
                 "$getField": {
-                    "input": "$$ROOT",
+                    "input": "$$scope_0",
                     "field": { "$literal": "$field" },
                 }
             }
@@ -289,16 +303,17 @@ mod tests {
 
     #[test]
     fn escapes_unsafe_nested_property_name_in_root_column_reference() -> anyhow::Result<()> {
-        let target = ComparisonTarget::RootCollectionColumn {
+        let target = ComparisonTarget::ColumnInScope {
             name: "field".into(),
             field_path: Some(vec!["$unsafe_name".into()]),
             column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
+            scope: Scope::Root,
         };
         let actual = ColumnRef::from_comparison_target(&target);
         let expected = ColumnRef::Expression(
             doc! {
                 "$getField": {
-                    "input": "$$ROOT.field",
+                    "input": "$$scope_root.field",
                     "field": { "$literal": "$unsafe_name" },
                 }
             }
@@ -311,10 +326,11 @@ mod tests {
     #[test]
     fn escapes_multiple_layers_of_nested_property_names_in_root_column_reference(
     ) -> anyhow::Result<()> {
-        let target = ComparisonTarget::RootCollectionColumn {
+        let target = ComparisonTarget::ColumnInScope {
             name: "$field".into(),
             field_path: Some(vec!["$unsafe_name1".into(), "$unsafe_name2".into()]),
             column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
+            scope: Scope::Root,
         };
         let actual = ColumnRef::from_comparison_target(&target);
         let expected = ColumnRef::Expression(
@@ -324,7 +340,7 @@ mod tests {
                         "$getField": {
                             "input": {
                                 "$getField": {
-                                    "input": "$$ROOT",
+                                    "input": "$$scope_root",
                                     "field": { "$literal": "$field" },
                                 }
                             },
@@ -342,16 +358,17 @@ mod tests {
 
     #[test]
     fn escapes_unsafe_deeply_nested_property_name_in_root_column_reference() -> anyhow::Result<()> {
-        let target = ComparisonTarget::RootCollectionColumn {
+        let target = ComparisonTarget::ColumnInScope {
             name: "field".into(),
             field_path: Some(vec!["prop1".into(), "$unsafe_name".into()]),
             column_type: Type::Scalar(MongoScalarType::Bson(BsonScalarType::String)),
+            scope: Scope::Root,
         };
         let actual = ColumnRef::from_comparison_target(&target);
         let expected = ColumnRef::Expression(
             doc! {
                 "$getField": {
-                    "input": "$$ROOT.field.prop1",
+                    "input": "$$scope_root.field.prop1",
                     "field": { "$literal": "$unsafe_name" },
                 }
             }