add config option for relaxed or canonical extended json output (#84)

Adds an option to allow users to opt into "relaxed" mode for Extended JSON output. Keeps "canonical" mode as the default because it is lossless. For example relaxed mode does not preserve the exact numeric type of each numeric value, while canonical mode does. This does not affect inputs. For example sorts and filters will accept either canonical or relaxed input modes as before. [MDB-169](https://hasurahq.atlassian.net/browse/MDB-169)
hasura · Jul 8, 2024 · 4bb84aa · 4bb84aa
1 parent 6a5e208
commit 4bb84aa
Show file tree

Hide file tree

Showing 12 changed files with 188 additions and 43 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@ This changelog documents the changes between release versions.
 - Rework query plans for requests with variable sets to allow use of indexes ([#83](https://github.com/hasura/ndc-mongodb/pull/83))
 - Fix: error when requesting query plan if MongoDB is target of a remote join ([#83](https://github.com/hasura/ndc-mongodb/pull/83))
 - Breaking change: remote joins no longer work in MongoDB v5 ([#83](https://github.com/hasura/ndc-mongodb/pull/83))
+- Add configuration option to opt into "relaxed" mode for Extended JSON outputs
+  ([#84](https://github.com/hasura/ndc-mongodb/pull/84))
 
 ## [0.1.0] - 2024-06-13
 

diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs
@@ -2,6 +2,7 @@ use std::{collections::BTreeMap, path::Path};
 
 use anyhow::{anyhow, ensure};
 use itertools::Itertools;
+use mongodb_support::ExtendedJsonMode;
 use ndc_models as ndc;
 use serde::{Deserialize, Serialize};
 
@@ -189,11 +190,16 @@ impl Configuration {
     }
 }
 
-#[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)]
+#[derive(Clone, Debug, Default, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ConfigurationOptions {
-    // Options for introspection
+    /// Options for introspection
     pub introspection_options: ConfigurationIntrospectionOptions,
+
+    /// Options that affect how BSON data from MongoDB is translated to JSON in GraphQL query
+    /// responses.
+    #[serde(default)]
+    pub serialization_options: ConfigurationSerializationOptions,
 }
 
 #[derive(Copy, Clone, Debug, Deserialize, Serialize)]
@@ -219,6 +225,15 @@ impl Default for ConfigurationIntrospectionOptions {
     }
 }
 
+#[derive(Clone, Debug, Default, Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ConfigurationSerializationOptions {
+    /// Extended JSON has two modes: canonical and relaxed. This option determines which mode is
+    /// used for output. This setting has no effect on inputs (query arguments, etc.).
+    #[serde(default)]
+    pub extended_json_mode: ExtendedJsonMode,
+}
+
 fn merge_object_types<'a>(
     schema: &'a serialized::Schema,
     native_mutations: &'a BTreeMap<String, serialized::NativeMutation>,

diff --git a/crates/mongodb-agent-common/proptest-regressions/query/serialization/tests.txt b/crates/mongodb-agent-common/proptest-regressions/query/serialization/tests.txt
@@ -9,3 +9,4 @@ cc 26e2543468ab6d4ffa34f9f8a2c920801ef38a35337557a8f4e74c92cf57e344 # shrinks to
 cc 7d760e540b56fedac7dd58e5bdb5bb9613b9b0bc6a88acfab3fc9c2de8bf026d # shrinks to bson = Document({"A": Array([Null, Undefined])})
 cc 21360610045c5a616b371fb8d5492eb0c22065d62e54d9c8a8761872e2e192f3 # shrinks to bson = Array([Document({}), Document({" ": Null})])
 cc 8842e7f78af24e19847be5d8ee3d47c547ef6c1bb54801d360a131f41a87f4fa
+cc 2a192b415e5669716701331fe4141383a12ceda9acc9f32e4284cbc2ed6f2d8a # shrinks to bson = Document({"A": Document({"¡": JavaScriptCodeWithScope { code: "", scope: Document({"\0": Int32(-1)}) }})}), mode = Relaxed
diff --git a/crates/mongodb-agent-common/src/mongo_query_plan/mod.rs b/crates/mongodb-agent-common/src/mongo_query_plan/mod.rs
@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
 use configuration::{
     native_mutation::NativeMutation, native_query::NativeQuery, Configuration, MongoScalarType,
 };
-use mongodb_support::EXTENDED_JSON_TYPE_NAME;
+use mongodb_support::{ExtendedJsonMode, EXTENDED_JSON_TYPE_NAME};
 use ndc_models as ndc;
 use ndc_query_plan::{ConnectorTypes, QueryContext, QueryPlanError};
 
@@ -17,6 +17,10 @@ pub use ndc_query_plan::OrderByTarget;
 pub struct MongoConfiguration(pub Configuration);
 
 impl MongoConfiguration {
+    pub fn extended_json_mode(&self) -> ExtendedJsonMode {
+        self.0.options.serialization_options.extended_json_mode
+    }
+
     pub fn native_queries(&self) -> &BTreeMap<String, NativeQuery> {
         &self.0.native_queries
     }

diff --git a/crates/mongodb-agent-common/src/query/execute_query_request.rs b/crates/mongodb-agent-common/src/query/execute_query_request.rs
@@ -27,7 +27,7 @@ pub async fn execute_query_request(
     let query_plan = preprocess_query_request(config, query_request)?;
     let pipeline = pipeline_for_query_request(config, &query_plan)?;
     let documents = execute_query_pipeline(database, config, &query_plan, pipeline).await?;
-    let response = serialize_query_response(&query_plan, documents)?;
+    let response = serialize_query_response(config.extended_json_mode(), &query_plan, documents)?;
     Ok(response)
 }
 

diff --git a/crates/mongodb-agent-common/src/query/response.rs b/crates/mongodb-agent-common/src/query/response.rs
@@ -4,6 +4,7 @@ use configuration::MongoScalarType;
 use indexmap::IndexMap;
 use itertools::Itertools;
 use mongodb::bson::{self, Bson};
+use mongodb_support::ExtendedJsonMode;
 use ndc_models::{QueryResponse, RowFieldValue, RowSet};
 use serde::Deserialize;
 use thiserror::Error;
@@ -49,6 +50,7 @@ struct BsonRowSet {
 
 #[instrument(name = "Serialize Query Response", skip_all, fields(internal.visibility = "user"))]
 pub fn serialize_query_response(
+    mode: ExtendedJsonMode,
     query_plan: &QueryPlan,
     response_documents: Vec<bson::Document>,
 ) -> Result<QueryResponse> {
@@ -59,18 +61,25 @@ pub fn serialize_query_response(
             .into_iter()
             .map(|document| {
                 let row_set = bson::from_document(document)?;
-                serialize_row_set_with_aggregates(&[collection_name], &query_plan.query, row_set)
+                serialize_row_set_with_aggregates(
+                    mode,
+                    &[collection_name],
+                    &query_plan.query,
+                    row_set,
+                )
             })
             .try_collect()
     } else if query_plan.query.has_aggregates() {
         let row_set = parse_single_document(response_documents)?;
         Ok(vec![serialize_row_set_with_aggregates(
+            mode,
             &[],
             &query_plan.query,
             row_set,
         )?])
     } else {
         Ok(vec![serialize_row_set_rows_only(
+            mode,
             &[],
             &query_plan.query,
             response_documents,
@@ -83,14 +92,15 @@ pub fn serialize_query_response(
 
 // When there are no aggregates we expect a list of rows
 fn serialize_row_set_rows_only(
+    mode: ExtendedJsonMode,
     path: &[&str],
     query: &Query,
     docs: Vec<bson::Document>,
 ) -> Result<RowSet> {
     let rows = query
         .fields
         .as_ref()
-        .map(|fields| serialize_rows(path, fields, docs))
+        .map(|fields| serialize_rows(mode, path, fields, docs))
         .transpose()?;
 
     Ok(RowSet {
@@ -102,32 +112,34 @@ fn serialize_row_set_rows_only(
 // When there are aggregates we expect a single document with `rows` and `aggregates`
 // fields
 fn serialize_row_set_with_aggregates(
+    mode: ExtendedJsonMode,
     path: &[&str],
     query: &Query,
     row_set: BsonRowSet,
 ) -> Result<RowSet> {
     let aggregates = query
         .aggregates
         .as_ref()
-        .map(|aggregates| serialize_aggregates(path, aggregates, row_set.aggregates))
+        .map(|aggregates| serialize_aggregates(mode, path, aggregates, row_set.aggregates))
         .transpose()?;
 
     let rows = query
         .fields
         .as_ref()
-        .map(|fields| serialize_rows(path, fields, row_set.rows))
+        .map(|fields| serialize_rows(mode, path, fields, row_set.rows))
         .transpose()?;
 
     Ok(RowSet { aggregates, rows })
 }
 
 fn serialize_aggregates(
+    mode: ExtendedJsonMode,
     path: &[&str],
     _query_aggregates: &IndexMap<String, Aggregate>,
     value: Bson,
 ) -> Result<IndexMap<String, serde_json::Value>> {
     let aggregates_type = type_for_aggregates()?;
-    let json = bson_to_json(&aggregates_type, value)?;
+    let json = bson_to_json(mode, &aggregates_type, value)?;
 
     // The NDC type uses an IndexMap for aggregate values; we need to convert the map
     // underlying the Value::Object value to an IndexMap
@@ -141,6 +153,7 @@ fn serialize_aggregates(
 }
 
 fn serialize_rows(
+    mode: ExtendedJsonMode,
     path: &[&str],
     query_fields: &IndexMap<String, Field>,
     docs: Vec<bson::Document>,
@@ -149,7 +162,7 @@ fn serialize_rows(
 
     docs.into_iter()
         .map(|doc| {
-            let json = bson_to_json(&row_type, doc.into())?;
+            let json = bson_to_json(mode, &row_type, doc.into())?;
             // The NDC types use an IndexMap for each row value; we need to convert the map
             // underlying the Value::Object value to an IndexMap
             let index_map = match json {
@@ -292,7 +305,7 @@ mod tests {
 
     use configuration::{Configuration, MongoScalarType};
     use mongodb::bson::{self, Bson};
-    use mongodb_support::BsonScalarType;
+    use mongodb_support::{BsonScalarType, ExtendedJsonMode};
     use ndc_models::{QueryRequest, QueryResponse, RowFieldValue, RowSet};
     use ndc_query_plan::plan_for_query_request;
     use ndc_test_helpers::{
@@ -331,7 +344,8 @@ mod tests {
             },
         }];
 
-        let response = serialize_query_response(&query_plan, response_documents)?;
+        let response =
+            serialize_query_response(ExtendedJsonMode::Canonical, &query_plan, response_documents)?;
         assert_eq!(
             response,
             QueryResponse(vec![RowSet {
@@ -370,7 +384,8 @@ mod tests {
             ],
         }];
 
-        let response = serialize_query_response(&query_plan, response_documents)?;
+        let response =
+            serialize_query_response(ExtendedJsonMode::Canonical, &query_plan, response_documents)?;
         assert_eq!(
             response,
             QueryResponse(vec![RowSet {
@@ -416,7 +431,8 @@ mod tests {
             },
         }];
 
-        let response = serialize_query_response(&query_plan, response_documents)?;
+        let response =
+            serialize_query_response(ExtendedJsonMode::Canonical, &query_plan, response_documents)?;
         assert_eq!(
             response,
             QueryResponse(vec![RowSet {
@@ -474,7 +490,8 @@ mod tests {
             "price_extjson": Bson::Decimal128(bson::Decimal128::from_str("-4.9999999999").unwrap()),
         }];
 
-        let response = serialize_query_response(&query_plan, response_documents)?;
+        let response =
+            serialize_query_response(ExtendedJsonMode::Canonical, &query_plan, response_documents)?;
         assert_eq!(
             response,
             QueryResponse(vec![RowSet {
@@ -531,7 +548,8 @@ mod tests {
             },
         }];
 
-        let response = serialize_query_response(&query_plan, response_documents)?;
+        let response =
+            serialize_query_response(ExtendedJsonMode::Canonical, &query_plan, response_documents)?;
         assert_eq!(
             response,
             QueryResponse(vec![RowSet {
@@ -556,6 +574,69 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn serializes_response_with_nested_extjson_in_relaxed_mode() -> anyhow::Result<()> {
+        let query_context = MongoConfiguration(Configuration {
+            collections: [collection("data")].into(),
+            object_types: [(
+                "data".into(),
+                object_type([("value", named_type("ExtendedJSON"))]),
+            )]
+            .into(),
+            functions: Default::default(),
+            procedures: Default::default(),
+            native_mutations: Default::default(),
+            native_queries: Default::default(),
+            options: Default::default(),
+        });
+
+        let request = query_request()
+            .collection("data")
+            .query(query().fields([field!("value")]))
+            .into();
+
+        let query_plan = plan_for_query_request(&query_context, request)?;
+
+        let response_documents = vec![bson::doc! {
+            "value": {
+                "array": [
+                    { "number": Bson::Int32(3) },
+                    { "number": Bson::Decimal128(bson::Decimal128::from_str("127.6486654").unwrap()) },
+                ],
+                "string": "hello",
+                "object": {
+                    "foo": 1,
+                    "bar": 2,
+                },
+            },
+        }];
+
+        let response =
+            serialize_query_response(ExtendedJsonMode::Relaxed, &query_plan, response_documents)?;
+        assert_eq!(
+            response,
+            QueryResponse(vec![RowSet {
+                aggregates: Default::default(),
+                rows: Some(vec![[(
+                    "value".into(),
+                    RowFieldValue(json!({
+                        "array": [
+                            { "number": 3 },
+                            { "number": { "$numberDecimal": "127.6486654" } },
+                        ],
+                        "string": "hello",
+                        "object": {
+                            "foo": 1,
+                            "bar": 2,
+                        },
+                    }))
+                )]
+                .into()]),
+            }])
+        );
+        Ok(())
+    }
+
     #[test]
     fn uses_field_path_to_guarantee_distinct_type_names() -> anyhow::Result<()> {
         let collection_name = "appearances";