hasura · codedmart · May 1, 2024 · Apr 30, 2024 · Apr 30, 2024 · Apr 30, 2024
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,9 @@ This changelog documents the changes between release versions.
 - Enables logging events from the MongoDB driver by setting the `RUST_LOG` variable ([#67](https://github.com/hasura/ndc-mongodb/pull/67))
   - To log all events set `RUST_LOG=mongodb::command=debug,mongodb::connection=debug,mongodb::server_selection=debug,mongodb::topology=debug`
 - Relations with a single column mapping now use concise correlated subquery syntax in `$lookup` stage ([#65](https://github.com/hasura/ndc-mongodb/pull/65))
+- Add root `configuration.json` or `configuration.yaml` file to allow editing cli options. ([#68](https://github.com/hasura/ndc-mongodb/pull/68))
+- Update default sample size to 100. ([#68](https://github.com/hasura/ndc-mongodb/pull/68))
+- Add `all_schema_nullable` option defaulted to true. ([#68](https://github.com/hasura/ndc-mongodb/pull/68))
 
 ## [0.0.5] - 2024-04-26
 - Fix incorrect order of results for query requests with more than 10 variable sets (#37)

diff --git a/crates/cli/src/introspection/sampling.rs b/crates/cli/src/introspection/sampling.rs
@@ -1,6 +1,6 @@
 use std::collections::{BTreeMap, HashSet};
 
-use super::type_unification::{unify_object_types, unify_type};
+use super::type_unification::{make_nullable_field, unify_object_types, unify_type};
 use configuration::{
     schema::{self, Type},
     Schema, WithName,
@@ -19,6 +19,8 @@ type ObjectType = WithName<schema::ObjectType>;
 /// are not unifiable.
 pub async fn sample_schema_from_db(
     sample_size: u32,
+    all_schema_nullalble: bool,
+    config_file_changed: bool,
     state: &ConnectorState,
     existing_schemas: &HashSet<std::string::String>,
 ) -> anyhow::Result<BTreeMap<std::string::String, Schema>> {
@@ -28,9 +30,9 @@ pub async fn sample_schema_from_db(
 
     while let Some(collection_spec) = collections_cursor.try_next().await? {
         let collection_name = collection_spec.name;
-        if !existing_schemas.contains(&collection_name) {
+        if !existing_schemas.contains(&collection_name) || config_file_changed {
             let collection_schema =
-                sample_schema_from_collection(&collection_name, sample_size, state).await?;
+                sample_schema_from_collection(&collection_name, sample_size, all_schema_nullalble, state).await?;
             schemas.insert(collection_name, collection_schema);
         }
     }
@@ -40,6 +42,7 @@ pub async fn sample_schema_from_db(
 async fn sample_schema_from_collection(
     collection_name: &str,
     sample_size: u32,
+    all_schema_nullalble: bool,
     state: &ConnectorState,
 ) -> anyhow::Result<Schema> {
     let db = state.database();
@@ -50,7 +53,7 @@ async fn sample_schema_from_collection(
         .await?;
     let mut collected_object_types = vec![];
     while let Some(document) = cursor.try_next().await? {
-        let object_types = make_object_type(collection_name, &document);
+        let object_types = make_object_type(collection_name, &document, all_schema_nullalble);
         collected_object_types = if collected_object_types.is_empty() {
             object_types
         } else {
@@ -71,13 +74,13 @@ async fn sample_schema_from_collection(
     })
 }
 
-fn make_object_type(object_type_name: &str, document: &Document) -> Vec<ObjectType> {
+fn make_object_type(object_type_name: &str, document: &Document, all_schema_nullalble: bool) -> Vec<ObjectType> {
     let (mut object_type_defs, object_fields) = {
         let type_prefix = format!("{object_type_name}_");
         let (object_type_defs, object_fields): (Vec<Vec<ObjectType>>, Vec<ObjectField>) = document
             .iter()
             .map(|(field_name, field_value)| {
-                make_object_field(&type_prefix, field_name, field_value)
+                make_object_field(&type_prefix, field_name, field_value, all_schema_nullalble)
             })
             .unzip();
         (object_type_defs.concat(), object_fields)
@@ -99,17 +102,22 @@ fn make_object_field(
     type_prefix: &str,
     field_name: &str,
     field_value: &Bson,
+    all_schema_nullalble: bool,
 ) -> (Vec<ObjectType>, ObjectField) {
     let object_type_name = format!("{type_prefix}{field_name}");
-    let (collected_otds, field_type) = make_field_type(&object_type_name, field_value);
-
-    let object_field = WithName::named(
+    let (collected_otds, field_type) = make_field_type(&object_type_name, field_value, all_schema_nullalble);
+    let object_field_value = WithName::named(
         field_name.to_owned(),
         schema::ObjectField {
             description: None,
             r#type: field_type,
         },
     );
+    let object_field = if all_schema_nullalble {
+        make_nullable_field(object_field_value)
+    } else {
+        object_field_value
+    };
 
     (collected_otds, object_field)
 }
@@ -118,12 +126,13 @@ fn make_object_field(
 pub fn type_from_bson(
     object_type_name: &str,
     value: &Bson,
+    all_schema_nullalble: bool,
 ) -> (BTreeMap<std::string::String, schema::ObjectType>, Type) {
-    let (object_types, t) = make_field_type(object_type_name, value);
+    let (object_types, t) = make_field_type(object_type_name, value, all_schema_nullalble);
     (WithName::into_map(object_types), t)
 }
 
-fn make_field_type(object_type_name: &str, field_value: &Bson) -> (Vec<ObjectType>, Type) {
+fn make_field_type(object_type_name: &str, field_value: &Bson, all_schema_nullalble: bool) -> (Vec<ObjectType>, Type) {
     fn scalar(t: BsonScalarType) -> (Vec<ObjectType>, Type) {
         (vec![], Type::Scalar(t))
     }
@@ -135,7 +144,7 @@ fn make_field_type(object_type_name: &str, field_value: &Bson) -> (Vec<ObjectTyp
             let mut collected_otds = vec![];
             let mut result_type = Type::Scalar(Undefined);
             for elem in arr {
-                let (elem_collected_otds, elem_type) = make_field_type(object_type_name, elem);
+                let (elem_collected_otds, elem_type) = make_field_type(object_type_name, elem, all_schema_nullalble);
                 collected_otds = if collected_otds.is_empty() {
                     elem_collected_otds
                 } else {
@@ -146,7 +155,7 @@ fn make_field_type(object_type_name: &str, field_value: &Bson) -> (Vec<ObjectTyp
             (collected_otds, Type::ArrayOf(Box::new(result_type)))
         }
         Bson::Document(document) => {
-            let collected_otds = make_object_type(object_type_name, document);
+            let collected_otds = make_object_type(object_type_name, document, all_schema_nullalble);
             (collected_otds, Type::Object(object_type_name.to_owned()))
         }
         Bson::Boolean(_) => scalar(Bool),
@@ -186,7 +195,7 @@ mod tests {
     fn simple_doc() -> Result<(), anyhow::Error> {
         let object_name = "foo";
         let doc = doc! {"my_int": 1, "my_string": "two"};
-        let result = WithName::into_map::<BTreeMap<_, _>>(make_object_type(object_name, &doc));
+        let result = WithName::into_map::<BTreeMap<_, _>>(make_object_type(object_name, &doc, false));
 
         let expected = BTreeMap::from([(
             object_name.to_owned(),
@@ -220,7 +229,7 @@ mod tests {
     fn array_of_objects() -> Result<(), anyhow::Error> {
         let object_name = "foo";
         let doc = doc! {"my_array": [{"foo": 42, "bar": ""}, {"bar": "wut", "baz": 3.77}]};
-        let result = WithName::into_map::<BTreeMap<_, _>>(make_object_type(object_name, &doc));
+        let result = WithName::into_map::<BTreeMap<_, _>>(make_object_type(object_name, &doc, false));
 
         let expected = BTreeMap::from([
             (
@@ -280,7 +289,7 @@ mod tests {
     fn non_unifiable_array_of_objects() -> Result<(), anyhow::Error> {
         let object_name = "foo";
         let doc = doc! {"my_array": [{"foo": 42, "bar": ""}, {"bar": 17, "baz": 3.77}]};
-        let result = WithName::into_map::<BTreeMap<_, _>>(make_object_type(object_name, &doc));
+        let result = WithName::into_map::<BTreeMap<_, _>>(make_object_type(object_name, &doc, false));
 
         let expected = BTreeMap::from([
             (

diff --git a/crates/cli/src/introspection/type_unification.rs b/crates/cli/src/introspection/type_unification.rs
@@ -72,10 +72,11 @@ pub fn unify_type(type_a: Type, type_b: Type) -> Type {
         // Anything else gives ExtendedJSON
         (_, _) => Type::ExtendedJSON,
     };
+
     result_type.normalize_type()
 }
 
-fn make_nullable_field(field: ObjectField) -> ObjectField {
+pub fn make_nullable_field(field: ObjectField) -> ObjectField {
     WithName::named(
         field.name,
         schema::ObjectField {

diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs
@@ -12,11 +12,14 @@ use mongodb_agent_common::state::ConnectorState;
 
 #[derive(Debug, Clone, Parser)]
 pub struct UpdateArgs {
-    #[arg(long = "sample-size", value_name = "N", default_value_t = 10)]
-    sample_size: u32,
+    #[arg(long = "sample-size", value_name = "N", required = false)]
+    sample_size: Option<u32>,
 
-    #[arg(long = "no-validator-schema", default_value_t = false)]
-    no_validator_schema: bool,
+    #[arg(long = "no-validator-schema", required = false)]
+    no_validator_schema: Option<bool>,
+
+    #[arg(long = "all-schema-nullable", required = false)]
+    all_schema_nullable: Option<bool>,
 }
 
 /// The command invoked by the user.
@@ -41,15 +44,33 @@ pub async fn run(command: Command, context: &Context) -> anyhow::Result<()> {
 
 /// Update the configuration in the current directory by introspecting the database.
 async fn update(context: &Context, args: &UpdateArgs) -> anyhow::Result<()> {
-    if !args.no_validator_schema {
+    let configuration_options = configuration::parse_configuration_options_file(&context.path).await;
+    // Prefer arguments passed to cli, and fallback to the configuration file
+    let sample_size = match args.sample_size {
+        Some(size) => size,
+        None => configuration_options.introspection_options.sample_size
+    };
+    let no_validator_schema = match args.no_validator_schema {
+        Some(validator) => validator,
+        None => configuration_options.introspection_options.no_validator_schema
+    };
+    let all_schema_nullable = match args.all_schema_nullable {
+        Some(validator) => validator,
+        None => configuration_options.introspection_options.all_schema_nullable
+    };
+    let config_file_changed = configuration::get_config_file_changed(&context.path).await?;
+
+    if !no_validator_schema {
         let schemas_from_json_validation =
             introspection::get_metadata_from_validation_schema(&context.connector_state).await?;
         configuration::write_schema_directory(&context.path, schemas_from_json_validation).await?;
     }
 
     let existing_schemas = configuration::list_existing_schemas(&context.path).await?;
     let schemas_from_sampling = introspection::sample_schema_from_db(
-        args.sample_size,
+        sample_size,
+        all_schema_nullable,
+        config_file_changed,
         &context.connector_state,
         &existing_schemas,
     )

diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs
@@ -4,6 +4,7 @@ use anyhow::{anyhow, ensure};
 use itertools::Itertools;
 use mongodb_support::BsonScalarType;
 use ndc_models as ndc;
+use serde::{Deserialize, Serialize};
 
 use crate::{
     native_procedure::NativeProcedure,
@@ -45,13 +46,16 @@ pub struct Configuration {
     /// `native_queries/`, and `native_procedures/` subdirectories in the connector configuration
     /// directory.
     pub object_types: BTreeMap<String, schema::ObjectType>,
+
+    pub options: ConfigurationOptions,
 }
 
 impl Configuration {
     pub fn validate(
         schema: serialized::Schema,
         native_procedures: BTreeMap<String, serialized::NativeProcedure>,
         native_queries: BTreeMap<String, serialized::NativeQuery>,
+        options: ConfigurationOptions
     ) -> anyhow::Result<Self> {
         let object_types_iter = || merge_object_types(&schema, &native_procedures, &native_queries);
         let object_type_errors = {
@@ -153,11 +157,12 @@ impl Configuration {
             native_procedures: internal_native_procedures,
             native_queries: internal_native_queries,
             object_types,
+            options
         })
     }
 
     pub fn from_schema(schema: serialized::Schema) -> anyhow::Result<Self> {
-        Self::validate(schema, Default::default(), Default::default())
+        Self::validate(schema, Default::default(), Default::default(), Default::default())
     }
 
     pub async fn parse_configuration(
@@ -167,6 +172,36 @@ impl Configuration {
     }
 }
 
+#[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ConfigurationOptions {
+    // Options for introspection
+    pub introspection_options: ConfigurationIntrospectionOptions,
+}
+
+#[derive(Copy, Clone, Debug, Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
+pub struct ConfigurationIntrospectionOptions {
+    // For introspection how many documents should be sampled per collection.
+    pub sample_size: u32,
+
+    // Whether to try validator schema first if one exists.
+    pub no_validator_schema: bool,
+
+    // Default to setting all schema fields as nullable.
+    pub all_schema_nullable: bool,
+}
+
+impl Default for ConfigurationIntrospectionOptions {
+    fn default() -> Self {
+        ConfigurationIntrospectionOptions {
+            sample_size: 100,
+            no_validator_schema: false,
+            all_schema_nullable: true,
+        }
+    }
+}
+
 fn merge_object_types<'a>(
     schema: &'a serialized::Schema,
     native_procedures: &'a BTreeMap<String, serialized::NativeProcedure>,
@@ -350,7 +385,7 @@ mod tests {
         )]
         .into_iter()
         .collect();
-        let result = Configuration::validate(schema, native_procedures, Default::default());
+        let result = Configuration::validate(schema, native_procedures, Default::default(), Default::default());
         let error_msg = result.unwrap_err().to_string();
         assert!(error_msg.contains("multiple definitions"));
         assert!(error_msg.contains("Album"));