From 4ab13f20856da1b445efcad9ef89b89db3d00e7c Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Mon, 29 Apr 2024 18:58:47 -0600 Subject: [PATCH 01/11] Add root config and update cli default --- crates/cli/src/lib.rs | 23 +++++++++++----- crates/configuration/src/configuration.rs | 27 +++++++++++++++++-- crates/configuration/src/directory.rs | 23 ++++++++++++++-- crates/configuration/src/lib.rs | 1 + .../src/query/native_query.rs | 1 + 5 files changed, 65 insertions(+), 10 deletions(-) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 139db0e9..f4f2c279 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -12,11 +12,11 @@ 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, - #[arg(long = "no-validator-schema", default_value_t = false)] - no_validator_schema: bool, + #[arg(long = "no-validator-schema", required = false)] + no_validator_schema: Option, } /// The command invoked by the user. @@ -41,7 +41,18 @@ 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 config_file = 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 => config_file.sample_size + }; + let no_validator_schema = match args.no_validator_schema { + Some(validator) => validator, + None => config_file.no_validator_schema + }; + + 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?; @@ -49,7 +60,7 @@ async fn update(context: &Context, args: &UpdateArgs) -> anyhow::Result<()> { let existing_schemas = configuration::list_existing_schemas(&context.path).await?; let schemas_from_sampling = introspection::sample_schema_from_db( - args.sample_size, + sample_size, &context.connector_state, &existing_schemas, ) diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs index 808eff82..cca1357a 100644 --- 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; use crate::{ native_procedure::NativeProcedure, @@ -11,6 +12,24 @@ use crate::{ read_directory, schema, serialized, }; +#[derive(Clone, Debug, Deserialize)] +pub struct ConfigurationOptions { + // 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, +} + +impl Default for ConfigurationOptions { + fn default() -> Self { + ConfigurationOptions { + sample_size: 100, + no_validator_schema: false + } + } +} + #[derive(Clone, Debug, Default)] pub struct Configuration { /// Tracked collections from the configured MongoDB database. This includes real collections as @@ -45,6 +64,8 @@ pub struct Configuration { /// `native_queries/`, and `native_procedures/` subdirectories in the connector configuration /// directory. pub object_types: BTreeMap, + + pub options: ConfigurationOptions, } impl Configuration { @@ -52,6 +73,7 @@ impl Configuration { schema: serialized::Schema, native_procedures: BTreeMap, native_queries: BTreeMap, + options: ConfigurationOptions ) -> anyhow::Result { let object_types_iter = || merge_object_types(&schema, &native_procedures, &native_queries); let object_type_errors = { @@ -153,11 +175,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::validate(schema, Default::default(), Default::default()) + Self::validate(schema, Default::default(), Default::default(), Default::default()) } pub async fn parse_configuration( @@ -350,7 +373,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")); diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index 1e659561..a46e6d4c 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -9,11 +9,13 @@ use std::{ use tokio::fs; use tokio_stream::wrappers::ReadDirStream; -use crate::{serialized::Schema, with_name::WithName, Configuration}; +use crate::{configuration::ConfigurationOptions, serialized::Schema, with_name::WithName, Configuration}; pub const SCHEMA_DIRNAME: &str = "schema"; pub const NATIVE_PROCEDURES_DIRNAME: &str = "native_procedures"; pub const NATIVE_QUERIES_DIRNAME: &str = "native_queries"; +pub const CONFIURATION_OPTIONS_JSON: &str = "configuration.json"; +pub const CONFIURATION_OPTIONS_YAML: &str = "configuration.yaml"; pub const CONFIGURATION_EXTENSIONS: [(&str, FileFormat); 3] = [("json", JSON), ("yaml", YAML), ("yml", YAML)]; @@ -47,7 +49,10 @@ pub async fn read_directory( .await? .unwrap_or_default(); - Configuration::validate(schema, native_procedures, native_queries) + let options = parse_configuration_options_file(&dir) + .await; + + Configuration::validate(schema, native_procedures, native_queries, options) } /// Parse all files in a directory with one of the allowed configuration extensions according to @@ -108,6 +113,20 @@ where } } +pub async fn parse_configuration_options_file(dir: &Path) -> ConfigurationOptions { + let json_config_file = parse_config_file(&dir.join(CONFIURATION_OPTIONS_JSON), JSON).await; + if let Ok(config_options) = json_config_file { + return config_options + } + + let yaml_config_file = parse_config_file(&dir.join(CONFIURATION_OPTIONS_YAML), YAML).await; + if let Ok(config_options) = yaml_config_file { + return config_options + } + + return Default::default() +} + async fn parse_config_file(path: impl AsRef, format: FileFormat) -> anyhow::Result where for<'a> T: Deserialize<'a>, diff --git a/crates/configuration/src/lib.rs b/crates/configuration/src/lib.rs index c7c13e4f..232f4968 100644 --- a/crates/configuration/src/lib.rs +++ b/crates/configuration/src/lib.rs @@ -10,5 +10,6 @@ pub use crate::configuration::Configuration; pub use crate::directory::list_existing_schemas; pub use crate::directory::read_directory; pub use crate::directory::write_schema_directory; +pub use crate::directory::parse_configuration_options_file; pub use crate::serialized::Schema; pub use crate::with_name::{WithName, WithNameRef}; diff --git a/crates/mongodb-agent-common/src/query/native_query.rs b/crates/mongodb-agent-common/src/query/native_query.rs index 9657ce64..abdc51bd 100644 --- a/crates/mongodb-agent-common/src/query/native_query.rs +++ b/crates/mongodb-agent-common/src/query/native_query.rs @@ -193,6 +193,7 @@ mod tests { functions: Default::default(), procedures: Default::default(), native_procedures: Default::default(), + options: Default::default(), }; let request = query_request() From 261cddc8b2d5beb90cab9fc02395053a0e78dd22 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Mon, 29 Apr 2024 19:09:03 -0600 Subject: [PATCH 02/11] Update CHANGELOG --- CHANGELOG.md | 1 + sample_mflix_config/.configuration_metadata | 0 sample_mflix_config/configuration.json | 7 + sample_mflix_config/schema/comments.json | 44 +++ .../schema/embedded_movies.json | 317 +++++++++++++++++ sample_mflix_config/schema/movies.json | 318 ++++++++++++++++++ sample_mflix_config/schema/sessions.json | 29 ++ sample_mflix_config/schema/theaters.json | 90 +++++ sample_mflix_config/schema/users.json | 44 +++ 9 files changed, 850 insertions(+) create mode 100644 sample_mflix_config/.configuration_metadata create mode 100644 sample_mflix_config/configuration.json create mode 100644 sample_mflix_config/schema/comments.json create mode 100644 sample_mflix_config/schema/embedded_movies.json create mode 100644 sample_mflix_config/schema/movies.json create mode 100644 sample_mflix_config/schema/sessions.json create mode 100644 sample_mflix_config/schema/theaters.json create mode 100644 sample_mflix_config/schema/users.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 9af173d0..dfd0b511 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ 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 an optional root `configuration.json` or `configuration.yaml` file to allow editing cli options. Update default sample size to 100. ([#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/sample_mflix_config/.configuration_metadata b/sample_mflix_config/.configuration_metadata new file mode 100644 index 00000000..e69de29b diff --git a/sample_mflix_config/configuration.json b/sample_mflix_config/configuration.json new file mode 100644 index 00000000..34030b3c --- /dev/null +++ b/sample_mflix_config/configuration.json @@ -0,0 +1,7 @@ +{ + "introspection_options": { + "sample_size": 250, + "no_validator_schema": true, + "all_schema_nullable": false + } +} \ No newline at end of file diff --git a/sample_mflix_config/schema/comments.json b/sample_mflix_config/schema/comments.json new file mode 100644 index 00000000..bc8d022e --- /dev/null +++ b/sample_mflix_config/schema/comments.json @@ -0,0 +1,44 @@ +{ + "name": "comments", + "collections": { + "comments": { + "type": "comments" + } + }, + "objectTypes": { + "comments": { + "fields": { + "_id": { + "type": { + "scalar": "objectId" + } + }, + "date": { + "type": { + "scalar": "date" + } + }, + "email": { + "type": { + "scalar": "string" + } + }, + "movie_id": { + "type": { + "scalar": "objectId" + } + }, + "name": { + "type": { + "scalar": "string" + } + }, + "text": { + "type": { + "scalar": "string" + } + } + } + } + } +} \ No newline at end of file diff --git a/sample_mflix_config/schema/embedded_movies.json b/sample_mflix_config/schema/embedded_movies.json new file mode 100644 index 00000000..281249ce --- /dev/null +++ b/sample_mflix_config/schema/embedded_movies.json @@ -0,0 +1,317 @@ +{ + "name": "embedded_movies", + "collections": { + "embedded_movies": { + "type": "embedded_movies" + } + }, + "objectTypes": { + "embedded_movies": { + "fields": { + "_id": { + "type": { + "scalar": "objectId" + } + }, + "awards": { + "type": { + "object": "embedded_movies_awards" + } + }, + "cast": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "countries": { + "type": { + "arrayOf": { + "scalar": "string" + } + } + }, + "directors": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "fullplot": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "genres": { + "type": { + "arrayOf": { + "scalar": "string" + } + } + }, + "imdb": { + "type": { + "object": "embedded_movies_imdb" + } + }, + "languages": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "lastupdated": { + "type": { + "scalar": "string" + } + }, + "metacritic": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "num_mflix_comments": { + "type": { + "scalar": "int" + } + }, + "plot": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "plot_embedding": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "double" + } + } + } + }, + "poster": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "rated": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "released": { + "type": { + "nullable": { + "scalar": "date" + } + } + }, + "runtime": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "title": { + "type": { + "scalar": "string" + } + }, + "tomatoes": { + "type": { + "nullable": { + "object": "embedded_movies_tomatoes" + } + } + }, + "type": { + "type": { + "scalar": "string" + } + }, + "writers": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "year": { + "type": { + "scalar": "int" + } + } + } + }, + "embedded_movies_awards": { + "fields": { + "nominations": { + "type": { + "scalar": "int" + } + }, + "text": { + "type": { + "scalar": "string" + } + }, + "wins": { + "type": { + "scalar": "int" + } + } + } + }, + "embedded_movies_imdb": { + "fields": { + "id": { + "type": { + "scalar": "int" + } + }, + "rating": { + "type": { + "scalar": "double" + } + }, + "votes": { + "type": { + "scalar": "int" + } + } + } + }, + "embedded_movies_tomatoes": { + "fields": { + "boxOffice": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "consensus": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "critic": { + "type": { + "nullable": { + "object": "embedded_movies_tomatoes_critic" + } + } + }, + "dvd": { + "type": { + "nullable": { + "scalar": "date" + } + } + }, + "fresh": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "lastUpdated": { + "type": { + "scalar": "date" + } + }, + "production": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "rotten": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "viewer": { + "type": { + "object": "embedded_movies_tomatoes_viewer" + } + }, + "website": { + "type": { + "nullable": { + "scalar": "string" + } + } + } + } + }, + "embedded_movies_tomatoes_critic": { + "fields": { + "meter": { + "type": { + "scalar": "int" + } + }, + "numReviews": { + "type": { + "scalar": "int" + } + }, + "rating": { + "type": { + "scalar": "double" + } + } + } + }, + "embedded_movies_tomatoes_viewer": { + "fields": { + "meter": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "numReviews": { + "type": { + "scalar": "int" + } + }, + "rating": { + "type": { + "scalar": "double" + } + } + } + } + } +} \ No newline at end of file diff --git a/sample_mflix_config/schema/movies.json b/sample_mflix_config/schema/movies.json new file mode 100644 index 00000000..97817783 --- /dev/null +++ b/sample_mflix_config/schema/movies.json @@ -0,0 +1,318 @@ +{ + "name": "movies", + "collections": { + "movies": { + "type": "movies" + } + }, + "objectTypes": { + "movies": { + "fields": { + "_id": { + "type": { + "scalar": "objectId" + } + }, + "awards": { + "type": { + "object": "movies_awards" + } + }, + "cast": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "countries": { + "type": { + "arrayOf": { + "scalar": "string" + } + } + }, + "directors": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "fullplot": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "genres": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "imdb": { + "type": { + "object": "movies_imdb" + } + }, + "languages": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "lastupdated": { + "type": { + "scalar": "string" + } + }, + "metacritic": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "num_mflix_comments": { + "type": { + "scalar": "int" + } + }, + "plot": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "poster": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "rated": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "released": { + "type": { + "nullable": { + "scalar": "date" + } + } + }, + "runtime": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "title": { + "type": { + "scalar": "string" + } + }, + "tomatoes": { + "type": { + "nullable": { + "object": "movies_tomatoes" + } + } + }, + "type": { + "type": { + "scalar": "string" + } + }, + "writers": { + "type": { + "nullable": { + "arrayOf": { + "scalar": "string" + } + } + } + }, + "year": { + "type": { + "scalar": "int" + } + } + } + }, + "movies_awards": { + "fields": { + "nominations": { + "type": { + "scalar": "int" + } + }, + "text": { + "type": { + "scalar": "string" + } + }, + "wins": { + "type": { + "scalar": "int" + } + } + } + }, + "movies_imdb": { + "fields": { + "id": { + "type": { + "scalar": "int" + } + }, + "rating": { + "type": { + "scalar": "double" + } + }, + "votes": { + "type": { + "scalar": "int" + } + } + } + }, + "movies_tomatoes": { + "fields": { + "boxOffice": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "consensus": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "critic": { + "type": { + "nullable": { + "object": "movies_tomatoes_critic" + } + } + }, + "dvd": { + "type": { + "nullable": { + "scalar": "date" + } + } + }, + "fresh": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "lastUpdated": { + "type": { + "scalar": "date" + } + }, + "production": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "rotten": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "viewer": { + "type": { + "object": "movies_tomatoes_viewer" + } + }, + "website": { + "type": { + "nullable": { + "scalar": "string" + } + } + } + } + }, + "movies_tomatoes_critic": { + "fields": { + "meter": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "numReviews": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "rating": { + "type": { + "nullable": { + "scalar": "double" + } + } + } + } + }, + "movies_tomatoes_viewer": { + "fields": { + "meter": { + "type": { + "nullable": { + "scalar": "int" + } + } + }, + "numReviews": { + "type": { + "scalar": "int" + } + }, + "rating": { + "type": { + "nullable": { + "scalar": "double" + } + } + } + } + } + } +} \ No newline at end of file diff --git a/sample_mflix_config/schema/sessions.json b/sample_mflix_config/schema/sessions.json new file mode 100644 index 00000000..364b9050 --- /dev/null +++ b/sample_mflix_config/schema/sessions.json @@ -0,0 +1,29 @@ +{ + "name": "sessions", + "collections": { + "sessions": { + "type": "sessions" + } + }, + "objectTypes": { + "sessions": { + "fields": { + "_id": { + "type": { + "scalar": "objectId" + } + }, + "jwt": { + "type": { + "scalar": "string" + } + }, + "user_id": { + "type": { + "scalar": "string" + } + } + } + } + } +} \ No newline at end of file diff --git a/sample_mflix_config/schema/theaters.json b/sample_mflix_config/schema/theaters.json new file mode 100644 index 00000000..df44678b --- /dev/null +++ b/sample_mflix_config/schema/theaters.json @@ -0,0 +1,90 @@ +{ + "name": "theaters", + "collections": { + "theaters": { + "type": "theaters" + } + }, + "objectTypes": { + "theaters": { + "fields": { + "_id": { + "type": { + "scalar": "objectId" + } + }, + "location": { + "type": { + "object": "theaters_location" + } + }, + "theaterId": { + "type": { + "scalar": "int" + } + } + } + }, + "theaters_location": { + "fields": { + "address": { + "type": { + "object": "theaters_location_address" + } + }, + "geo": { + "type": { + "object": "theaters_location_geo" + } + } + } + }, + "theaters_location_address": { + "fields": { + "city": { + "type": { + "scalar": "string" + } + }, + "state": { + "type": { + "scalar": "string" + } + }, + "street1": { + "type": { + "scalar": "string" + } + }, + "street2": { + "type": { + "nullable": { + "scalar": "string" + } + } + }, + "zipcode": { + "type": { + "scalar": "string" + } + } + } + }, + "theaters_location_geo": { + "fields": { + "coordinates": { + "type": { + "arrayOf": { + "scalar": "double" + } + } + }, + "type": { + "type": { + "scalar": "string" + } + } + } + } + } +} \ No newline at end of file diff --git a/sample_mflix_config/schema/users.json b/sample_mflix_config/schema/users.json new file mode 100644 index 00000000..ec2b7149 --- /dev/null +++ b/sample_mflix_config/schema/users.json @@ -0,0 +1,44 @@ +{ + "name": "users", + "collections": { + "users": { + "type": "users" + } + }, + "objectTypes": { + "users": { + "fields": { + "_id": { + "type": { + "scalar": "objectId" + } + }, + "email": { + "type": { + "scalar": "string" + } + }, + "name": { + "type": { + "scalar": "string" + } + }, + "password": { + "type": { + "scalar": "string" + } + }, + "preferences": { + "type": { + "nullable": { + "object": "users_preferences" + } + } + } + } + }, + "users_preferences": { + "fields": {} + } + } +} \ No newline at end of file From 90fd5aae2fb641fd2c105062904a8c667d9cde57 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Tue, 30 Apr 2024 11:23:57 -0600 Subject: [PATCH 03/11] Feedback and write config file if none exists --- crates/configuration/src/configuration.rs | 4 ++-- crates/configuration/src/directory.rs | 14 +++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs index cca1357a..45dd8bb0 100644 --- a/crates/configuration/src/configuration.rs +++ b/crates/configuration/src/configuration.rs @@ -4,7 +4,7 @@ use anyhow::{anyhow, ensure}; use itertools::Itertools; use mongodb_support::BsonScalarType; use ndc_models as ndc; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use crate::{ native_procedure::NativeProcedure, @@ -12,7 +12,7 @@ use crate::{ read_directory, schema, serialized, }; -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct ConfigurationOptions { // For introspection how many documents should be sampled per collection. pub sample_size: u32, diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index a46e6d4c..52a5ea91 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -14,8 +14,7 @@ use crate::{configuration::ConfigurationOptions, serialized::Schema, with_name:: pub const SCHEMA_DIRNAME: &str = "schema"; pub const NATIVE_PROCEDURES_DIRNAME: &str = "native_procedures"; pub const NATIVE_QUERIES_DIRNAME: &str = "native_queries"; -pub const CONFIURATION_OPTIONS_JSON: &str = "configuration.json"; -pub const CONFIURATION_OPTIONS_YAML: &str = "configuration.yaml"; +pub const CONFIGURATION_OPTIONS_BASENAME: &str = "configuration"; pub const CONFIGURATION_EXTENSIONS: [(&str, FileFormat); 3] = [("json", JSON), ("yaml", YAML), ("yml", YAML)]; @@ -114,17 +113,22 @@ where } pub async fn parse_configuration_options_file(dir: &Path) -> ConfigurationOptions { - let json_config_file = parse_config_file(&dir.join(CONFIURATION_OPTIONS_JSON), JSON).await; + let json_filename = CONFIGURATION_OPTIONS_BASENAME.to_owned() + ".json"; + let json_config_file = parse_config_file(&dir.join(json_filename), JSON).await; if let Ok(config_options) = json_config_file { return config_options } - let yaml_config_file = parse_config_file(&dir.join(CONFIURATION_OPTIONS_YAML), YAML).await; + let yaml_filename = CONFIGURATION_OPTIONS_BASENAME.to_owned() + ".yaml"; + let yaml_config_file = parse_config_file(&dir.join(yaml_filename), YAML).await; if let Ok(config_options) = yaml_config_file { return config_options } - return Default::default() + // If a configuration file does not exist use defaults and write the file + let defaults: ConfigurationOptions = Default::default(); + let _ = write_file(dir, CONFIGURATION_OPTIONS_BASENAME, &defaults).await; + return defaults } async fn parse_config_file(path: impl AsRef, format: FileFormat) -> anyhow::Result From 360dd4528679de649dcaec3b3e333d35933a3955 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Tue, 30 Apr 2024 19:56:21 -0600 Subject: [PATCH 04/11] Added nullable by default config option --- crates/cli/src/introspection/sampling.rs | 37 +++-- .../cli/src/introspection/type_unification.rs | 138 ++++++++++-------- crates/cli/src/lib.rs | 10 ++ crates/configuration/src/configuration.rs | 8 +- crates/configuration/src/directory.rs | 56 ++++++- crates/configuration/src/lib.rs | 1 + .../src/query/serialization/tests.rs | 2 +- 7 files changed, 168 insertions(+), 84 deletions(-) diff --git a/crates/cli/src/introspection/sampling.rs b/crates/cli/src/introspection/sampling.rs index 86bce3c4..66234915 100644 --- a/crates/cli/src/introspection/sampling.rs +++ b/crates/cli/src/introspection/sampling.rs @@ -19,6 +19,8 @@ type ObjectType = WithName; /// 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, ) -> anyhow::Result> { @@ -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 { let db = state.database(); @@ -50,11 +53,11 @@ 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 { - unify_object_types(collected_object_types, object_types) + unify_object_types(collected_object_types, object_types, all_schema_nullalble) }; } let collection_info = WithName::named( @@ -71,13 +74,13 @@ async fn sample_schema_from_collection( }) } -fn make_object_type(object_type_name: &str, document: &Document) -> Vec { +fn make_object_type(object_type_name: &str, document: &Document, all_schema_nullalble: bool) -> Vec { let (mut object_type_defs, object_fields) = { let type_prefix = format!("{object_type_name}_"); let (object_type_defs, object_fields): (Vec>, Vec) = 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,9 +102,10 @@ fn make_object_field( type_prefix: &str, field_name: &str, field_value: &Bson, + all_schema_nullalble: bool, ) -> (Vec, ObjectField) { let object_type_name = format!("{type_prefix}{field_name}"); - let (collected_otds, field_type) = make_field_type(&object_type_name, field_value); + let (collected_otds, field_type) = make_field_type(&object_type_name, field_value, all_schema_nullalble); let object_field = WithName::named( field_name.to_owned(), @@ -118,12 +122,13 @@ fn make_object_field( pub fn type_from_bson( object_type_name: &str, value: &Bson, + all_schema_nullalble: bool, ) -> (BTreeMap, 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, Type) { +fn make_field_type(object_type_name: &str, field_value: &Bson, all_schema_nullalble: bool) -> (Vec, Type) { fn scalar(t: BsonScalarType) -> (Vec, Type) { (vec![], Type::Scalar(t)) } @@ -135,18 +140,18 @@ fn make_field_type(object_type_name: &str, field_value: &Bson) -> (Vec { - 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 +191,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::>(make_object_type(object_name, &doc)); + let result = WithName::into_map::>(make_object_type(object_name, &doc, false)); let expected = BTreeMap::from([( object_name.to_owned(), @@ -220,7 +225,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::>(make_object_type(object_name, &doc)); + let result = WithName::into_map::>(make_object_type(object_name, &doc, false)); let expected = BTreeMap::from([ ( @@ -280,7 +285,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::>(make_object_type(object_name, &doc)); + let result = WithName::into_map::>(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 index dae7f3fa..133d27b4 100644 --- a/crates/cli/src/introspection/type_unification.rs +++ b/crates/cli/src/introspection/type_unification.rs @@ -17,8 +17,8 @@ type ObjectType = WithName; /// Unify two types. /// This is computing the join (or least upper bound) of the two types in a lattice /// where `ExtendedJSON` is the Top element, Scalar(Undefined) is the Bottom element, and Nullable(T) >= T for all T. -pub fn unify_type(type_a: Type, type_b: Type) -> Type { - let result_type = match (type_a, type_b) { +pub fn unify_type(type_a: Type, type_b: Type, all_schema_nullable: bool) -> Type { + let pre_result_type = match (type_a, type_b) { // Union of any type with ExtendedJSON is ExtendedJSON (Type::ExtendedJSON, _) => Type::ExtendedJSON, (_, Type::ExtendedJSON) => Type::ExtendedJSON, @@ -31,11 +31,11 @@ pub fn unify_type(type_a: Type, type_b: Type) -> Type { // A Nullable type will unify with another type iff the underlying type is unifiable. // The resulting type will be Nullable. (Type::Nullable(nullable_type_a), type_b) => { - let result_type = unify_type(*nullable_type_a, type_b); + let result_type = unify_type(*nullable_type_a, type_b, all_schema_nullable); result_type.make_nullable() } (type_a, Type::Nullable(nullable_type_b)) => { - let result_type = unify_type(type_a, *nullable_type_b); + let result_type = unify_type(type_a, *nullable_type_b, all_schema_nullable); result_type.make_nullable() } @@ -65,13 +65,20 @@ pub fn unify_type(type_a: Type, type_b: Type) -> Type { // Array types unify iff their element types unify. (Type::ArrayOf(elem_type_a), Type::ArrayOf(elem_type_b)) => { - let elem_type = unify_type(*elem_type_a, *elem_type_b); + let elem_type = unify_type(*elem_type_a, *elem_type_b, all_schema_nullable); Type::ArrayOf(Box::new(elem_type)) } // Anything else gives ExtendedJSON (_, _) => Type::ExtendedJSON, }; + + let result_type = match (all_schema_nullable, &pre_result_type) { + (_, Type::Nullable(_)) => pre_result_type, + (true, _) => pre_result_type.make_nullable(), + _ => pre_result_type, + }; + result_type.normalize_type() } @@ -87,58 +94,62 @@ fn make_nullable_field(field: ObjectField) -> ObjectField { /// Unify two `ObjectType`s. /// Any field that appears in only one of the `ObjectType`s will be made nullable. -fn unify_object_type(object_type_a: ObjectType, object_type_b: ObjectType) -> ObjectType { - let field_map_a: IndexMap = object_type_a - .value - .fields - .into_iter() - .map_into::() - .map(|o| (o.name.to_owned(), o)) - .collect(); - let field_map_b: IndexMap = object_type_b - .value - .fields - .into_iter() - .map_into::() - .map(|o| (o.name.to_owned(), o)) - .collect(); +fn unify_object_type(all_schema_nullable: bool) -> impl Fn(ObjectType, ObjectType) -> ObjectType { + move |object_type_a, object_type_b| { + let field_map_a: IndexMap = object_type_a + .value + .fields + .into_iter() + .map_into::() + .map(|o| (o.name.to_owned(), o)) + .collect(); + let field_map_b: IndexMap = object_type_b + .value + .fields + .into_iter() + .map_into::() + .map(|o| (o.name.to_owned(), o)) + .collect(); + + let merged_field_map = align( + field_map_a, + field_map_b, + make_nullable_field, + make_nullable_field, + unify_object_field(all_schema_nullable), + ); - let merged_field_map = align( - field_map_a, - field_map_b, - make_nullable_field, - make_nullable_field, - unify_object_field, - ); + WithName::named( + object_type_a.name, + schema::ObjectType { + fields: merged_field_map + .into_values() + .map(WithName::into_name_value_pair) + .collect(), + description: object_type_a + .value + .description + .or(object_type_b.value.description), + }, + ) - WithName::named( - object_type_a.name, - schema::ObjectType { - fields: merged_field_map - .into_values() - .map(WithName::into_name_value_pair) - .collect(), - description: object_type_a - .value - .description - .or(object_type_b.value.description), - }, - ) + } } /// Unify the types of two `ObjectField`s. /// If the types are not unifiable then return an error. -fn unify_object_field(object_field_a: ObjectField, object_field_b: ObjectField) -> ObjectField { - WithName::named( - object_field_a.name, - schema::ObjectField { - r#type: unify_type(object_field_a.value.r#type, object_field_b.value.r#type), - description: object_field_a - .value - .description - .or(object_field_b.value.description), - }, - ) +fn unify_object_field(all_schema_nullable: bool) -> impl Fn(ObjectField, ObjectField) -> ObjectField { + move |object_field_a, object_field_b| + WithName::named( + object_field_a.name, + schema::ObjectField { + r#type: unify_type(object_field_a.value.r#type, object_field_b.value.r#type, all_schema_nullable), + description: object_field_a + .value + .description + .or(object_field_b.value.description), + }, + ) } /// Unify two sets of `ObjectType`s. @@ -147,6 +158,7 @@ fn unify_object_field(object_field_a: ObjectField, object_field_b: ObjectField) pub fn unify_object_types( object_types_a: Vec, object_types_b: Vec, + all_schema_nullable: bool, ) -> Vec { let type_map_a: IndexMap = object_types_a .into_iter() @@ -162,7 +174,7 @@ pub fn unify_object_types( type_map_b, std::convert::identity, std::convert::identity, - unify_object_type, + unify_object_type(all_schema_nullable), ); merged_type_map.into_values().collect() @@ -187,6 +199,7 @@ mod tests { let actual = unify_type( Type::Scalar(BsonScalarType::Int), Type::Scalar(BsonScalarType::Int), + false ); assert_eq!(expected, actual); Ok(()) @@ -198,6 +211,7 @@ mod tests { let actual = unify_type( Type::Scalar(BsonScalarType::Int), Type::Scalar(BsonScalarType::String), + false ); assert_eq!(expected, actual); Ok(()) @@ -213,7 +227,7 @@ mod tests { proptest! { #[test] fn test_type_unifies_with_itself_and_normalizes(t in arb_type()) { - let u = unify_type(t.clone(), t.clone()); + let u = unify_type(t.clone(), t.clone(), false); prop_assert_eq!(t.normalize_type(), u) } } @@ -221,8 +235,8 @@ mod tests { proptest! { #[test] fn test_unify_type_is_commutative(ta in arb_type(), tb in arb_type()) { - let result_a_b = unify_type(ta.clone(), tb.clone()); - let result_b_a = unify_type(tb, ta); + let result_a_b = unify_type(ta.clone(), tb.clone(), false); + let result_b_a = unify_type(tb, ta, false); prop_assert_eq!(result_a_b, result_b_a) } } @@ -230,8 +244,8 @@ mod tests { proptest! { #[test] fn test_unify_type_is_associative(ta in arb_type(), tb in arb_type(), tc in arb_type()) { - let result_lr = unify_type(unify_type(ta.clone(), tb.clone()), tc.clone()); - let result_rl = unify_type(ta, unify_type(tb, tc)); + let result_lr = unify_type(unify_type(ta.clone(), tb.clone(), false), tc.clone(), false); + let result_rl = unify_type(ta, unify_type(tb, tc, false), false); prop_assert_eq!(result_lr, result_rl) } } @@ -239,7 +253,7 @@ mod tests { proptest! { #[test] fn test_undefined_is_left_identity(t in arb_type()) { - let u = unify_type(Type::Scalar(BsonScalarType::Undefined), t.clone()); + let u = unify_type(Type::Scalar(BsonScalarType::Undefined), t.clone(), false); prop_assert_eq!(t.normalize_type(), u) } } @@ -247,7 +261,7 @@ mod tests { proptest! { #[test] fn test_undefined_is_right_identity(t in arb_type()) { - let u = unify_type(t.clone(), Type::Scalar(BsonScalarType::Undefined)); + let u = unify_type(t.clone(), Type::Scalar(BsonScalarType::Undefined), false); prop_assert_eq!(t.normalize_type(), u) } } @@ -255,7 +269,7 @@ mod tests { proptest! { #[test] fn test_any_left(t in arb_type()) { - let u = unify_type(Type::ExtendedJSON, t); + let u = unify_type(Type::ExtendedJSON, t, false); prop_assert_eq!(Type::ExtendedJSON, u) } } @@ -263,7 +277,7 @@ mod tests { proptest! { #[test] fn test_any_right(t in arb_type()) { - let u = unify_type(t, Type::ExtendedJSON); + let u = unify_type(t, Type::ExtendedJSON, false); prop_assert_eq!(Type::ExtendedJSON, u) } } @@ -291,7 +305,7 @@ mod tests { fields: right_fields.into_iter().map(|(k, v)| (k, schema::ObjectField{r#type: v, description: None})).collect(), description: None }); - let result = unify_object_type(left_object, right_object); + let result = unify_object_type(false)(left_object, right_object); for field in result.value.named_fields() { // Any fields not shared between the two input types should be nullable. diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index f4f2c279..c16361ba 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -17,6 +17,9 @@ pub struct UpdateArgs { #[arg(long = "no-validator-schema", required = false)] no_validator_schema: Option, + + #[arg(long = "all-schema-nullable", required = false)] + all_schema_nullable: Option, } /// The command invoked by the user. @@ -51,6 +54,11 @@ async fn update(context: &Context, args: &UpdateArgs) -> anyhow::Result<()> { Some(validator) => validator, None => config_file.no_validator_schema }; + let all_schema_nullable = match args.all_schema_nullable { + Some(validator) => validator, + None => config_file.all_schema_nullable + }; + let config_file_changed = configuration::get_config_file_changed(&context.path).await?; if !no_validator_schema { let schemas_from_json_validation = @@ -61,6 +69,8 @@ async fn update(context: &Context, args: &UpdateArgs) -> anyhow::Result<()> { let existing_schemas = configuration::list_existing_schemas(&context.path).await?; let schemas_from_sampling = introspection::sample_schema_from_db( 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 index 45dd8bb0..97cd3ce9 100644 --- a/crates/configuration/src/configuration.rs +++ b/crates/configuration/src/configuration.rs @@ -12,20 +12,24 @@ use crate::{ read_directory, schema, serialized, }; -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Copy, Clone, Debug, Deserialize, Serialize)] pub struct ConfigurationOptions { // 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 ConfigurationOptions { fn default() -> Self { ConfigurationOptions { sample_size: 100, - no_validator_schema: false + no_validator_schema: false, + all_schema_nullable: true, } } } diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index 52a5ea91..ff7dde5c 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -3,10 +3,9 @@ use futures::stream::TryStreamExt as _; use itertools::Itertools as _; use serde::{Deserialize, Serialize}; use std::{ - collections::{BTreeMap, HashSet}, - path::{Path, PathBuf}, + collections::{BTreeMap, HashSet}, fs::Metadata, path::{Path, PathBuf} }; -use tokio::fs; +use tokio::{fs, io::AsyncWriteExt}; use tokio_stream::wrappers::ReadDirStream; use crate::{configuration::ConfigurationOptions, serialized::Schema, with_name::WithName, Configuration}; @@ -15,6 +14,7 @@ pub const SCHEMA_DIRNAME: &str = "schema"; pub const NATIVE_PROCEDURES_DIRNAME: &str = "native_procedures"; pub const NATIVE_QUERIES_DIRNAME: &str = "native_queries"; pub const CONFIGURATION_OPTIONS_BASENAME: &str = "configuration"; +pub const CONFIGURATION_OPTIONS_METADATA: &str = ".configuration_metadata"; pub const CONFIGURATION_EXTENSIONS: [(&str, FileFormat); 3] = [("json", JSON), ("yaml", YAML), ("yml", YAML)]; @@ -128,6 +128,7 @@ pub async fn parse_configuration_options_file(dir: &Path) -> ConfigurationOption // If a configuration file does not exist use defaults and write the file let defaults: ConfigurationOptions = Default::default(); let _ = write_file(dir, CONFIGURATION_OPTIONS_BASENAME, &defaults).await; + let _ = write_config_metadata_file(dir).await; return defaults } @@ -211,3 +212,52 @@ pub async fn list_existing_schemas( Ok(schemas.into_keys().collect()) } + +async fn write_config_metadata_file( + configuration_dir: impl AsRef +) -> () { + let dir = configuration_dir.as_ref(); + let file_result = fs::OpenOptions::new() + .write(true) + .truncate(true) + .create(true) + .open(dir.join(CONFIGURATION_OPTIONS_METADATA)) + .await; + match file_result { + Ok(mut file) => { + let _ = file.write_all(b"").await; + }, + Err(_) => () + }; +} + +pub async fn get_config_file_changed( + dir: impl AsRef +) -> anyhow::Result { + let path = dir.as_ref(); + let dot_metadata: Result = fs::metadata( + &path.join(CONFIGURATION_OPTIONS_METADATA) + ).await; + let json_metadata = fs::metadata( + &path.join(CONFIGURATION_OPTIONS_BASENAME.to_owned() + ".json") + ).await; + let yaml_metadata = fs::metadata( + &path.join(CONFIGURATION_OPTIONS_BASENAME.to_owned() + ".yaml") + ).await; + + let compare = |dot_date, config_date| async move { + if dot_date < config_date { + let _ = write_config_metadata_file(path).await; + Ok(true) + } + else { + Ok(false) + } + }; + + match (dot_metadata, json_metadata, yaml_metadata) { + (Ok(dot), Ok(json), _) => compare(dot.modified()?, json.modified()?).await, + (Ok(dot), _, Ok(yaml)) => compare(dot.modified()?, yaml.modified()?).await, + _ => Ok(true) + } +} diff --git a/crates/configuration/src/lib.rs b/crates/configuration/src/lib.rs index 232f4968..9a99aa3d 100644 --- a/crates/configuration/src/lib.rs +++ b/crates/configuration/src/lib.rs @@ -11,5 +11,6 @@ pub use crate::directory::list_existing_schemas; pub use crate::directory::read_directory; pub use crate::directory::write_schema_directory; pub use crate::directory::parse_configuration_options_file; +pub use crate::directory::get_config_file_changed; pub use crate::serialized::Schema; pub use crate::with_name::{WithName, WithNameRef}; diff --git a/crates/mongodb-agent-common/src/query/serialization/tests.rs b/crates/mongodb-agent-common/src/query/serialization/tests.rs index e6eb52eb..79ace254 100644 --- a/crates/mongodb-agent-common/src/query/serialization/tests.rs +++ b/crates/mongodb-agent-common/src/query/serialization/tests.rs @@ -10,7 +10,7 @@ use super::{bson_to_json, json_to_bson}; proptest! { #[test] fn converts_bson_to_json_and_back(bson in arb_bson()) { - let (object_types, inferred_type) = type_from_bson("test_object", &bson); + let (object_types, inferred_type) = type_from_bson("test_object", &bson, false); let error_context = |msg: &str, source: String| TestCaseError::fail(format!("{msg}: {source}\ninferred type: {inferred_type:?}\nobject types: {object_types:?}")); let json = bson_to_json(&inferred_type, &object_types, bson.clone()).map_err(|e| error_context("error converting bson to json", e.to_string()))?; let actual = json_to_bson(&inferred_type, &object_types, json.clone()).map_err(|e| error_context("error converting json to bson", e.to_string()))?; From ce0835762aff703ccdfef9e4ddc358257f5662d8 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 1 May 2024 07:16:05 -0600 Subject: [PATCH 05/11] Review feedback --- CHANGELOG.md | 3 + crates/cli/src/introspection/sampling.rs | 16 +- .../cli/src/introspection/type_unification.rs | 141 ++++++++---------- crates/cli/src/lib.rs | 8 +- crates/configuration/src/configuration.rs | 50 ++++--- 5 files changed, 109 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfd0b511..0a8bdbb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ This changelog documents the changes between release versions. - 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 an optional root `configuration.json` or `configuration.yaml` file to allow editing cli options. Update default sample size to 100. ([#68](https://github.com/hasura/ndc-mongodb/pull/68)) +- 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 index 66234915..51dc41f9 100644 --- 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, @@ -57,7 +57,7 @@ async fn sample_schema_from_collection( collected_object_types = if collected_object_types.is_empty() { object_types } else { - unify_object_types(collected_object_types, object_types, all_schema_nullalble) + unify_object_types(collected_object_types, object_types) }; } let collection_info = WithName::named( @@ -106,14 +106,18 @@ fn make_object_field( ) -> (Vec, ObjectField) { let object_type_name = format!("{type_prefix}{field_name}"); let (collected_otds, field_type) = make_field_type(&object_type_name, field_value, all_schema_nullalble); - - let object_field = WithName::named( + 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) } @@ -144,9 +148,9 @@ fn make_field_type(object_type_name: &str, field_value: &Bson, all_schema_nullal collected_otds = if collected_otds.is_empty() { elem_collected_otds } else { - unify_object_types(collected_otds, elem_collected_otds, all_schema_nullalble) + unify_object_types(collected_otds, elem_collected_otds) }; - result_type = unify_type(result_type, elem_type, all_schema_nullalble); + result_type = unify_type(result_type, elem_type); } (collected_otds, Type::ArrayOf(Box::new(result_type))) } diff --git a/crates/cli/src/introspection/type_unification.rs b/crates/cli/src/introspection/type_unification.rs index 133d27b4..54727fe8 100644 --- a/crates/cli/src/introspection/type_unification.rs +++ b/crates/cli/src/introspection/type_unification.rs @@ -17,8 +17,8 @@ type ObjectType = WithName; /// Unify two types. /// This is computing the join (or least upper bound) of the two types in a lattice /// where `ExtendedJSON` is the Top element, Scalar(Undefined) is the Bottom element, and Nullable(T) >= T for all T. -pub fn unify_type(type_a: Type, type_b: Type, all_schema_nullable: bool) -> Type { - let pre_result_type = match (type_a, type_b) { +pub fn unify_type(type_a: Type, type_b: Type) -> Type { + let result_type = match (type_a, type_b) { // Union of any type with ExtendedJSON is ExtendedJSON (Type::ExtendedJSON, _) => Type::ExtendedJSON, (_, Type::ExtendedJSON) => Type::ExtendedJSON, @@ -31,11 +31,11 @@ pub fn unify_type(type_a: Type, type_b: Type, all_schema_nullable: bool) -> Type // A Nullable type will unify with another type iff the underlying type is unifiable. // The resulting type will be Nullable. (Type::Nullable(nullable_type_a), type_b) => { - let result_type = unify_type(*nullable_type_a, type_b, all_schema_nullable); + let result_type = unify_type(*nullable_type_a, type_b); result_type.make_nullable() } (type_a, Type::Nullable(nullable_type_b)) => { - let result_type = unify_type(type_a, *nullable_type_b, all_schema_nullable); + let result_type = unify_type(type_a, *nullable_type_b); result_type.make_nullable() } @@ -65,7 +65,7 @@ pub fn unify_type(type_a: Type, type_b: Type, all_schema_nullable: bool) -> Type // Array types unify iff their element types unify. (Type::ArrayOf(elem_type_a), Type::ArrayOf(elem_type_b)) => { - let elem_type = unify_type(*elem_type_a, *elem_type_b, all_schema_nullable); + let elem_type = unify_type(*elem_type_a, *elem_type_b); Type::ArrayOf(Box::new(elem_type)) } @@ -73,16 +73,10 @@ pub fn unify_type(type_a: Type, type_b: Type, all_schema_nullable: bool) -> Type (_, _) => Type::ExtendedJSON, }; - let result_type = match (all_schema_nullable, &pre_result_type) { - (_, Type::Nullable(_)) => pre_result_type, - (true, _) => pre_result_type.make_nullable(), - _ => pre_result_type, - }; - - result_type.normalize_type() + result_type } -fn make_nullable_field(field: ObjectField) -> ObjectField { +pub fn make_nullable_field(field: ObjectField) -> ObjectField { WithName::named( field.name, schema::ObjectField { @@ -94,62 +88,58 @@ fn make_nullable_field(field: ObjectField) -> ObjectField { /// Unify two `ObjectType`s. /// Any field that appears in only one of the `ObjectType`s will be made nullable. -fn unify_object_type(all_schema_nullable: bool) -> impl Fn(ObjectType, ObjectType) -> ObjectType { - move |object_type_a, object_type_b| { - let field_map_a: IndexMap = object_type_a - .value - .fields - .into_iter() - .map_into::() - .map(|o| (o.name.to_owned(), o)) - .collect(); - let field_map_b: IndexMap = object_type_b - .value - .fields - .into_iter() - .map_into::() - .map(|o| (o.name.to_owned(), o)) - .collect(); - - let merged_field_map = align( - field_map_a, - field_map_b, - make_nullable_field, - make_nullable_field, - unify_object_field(all_schema_nullable), - ); +fn unify_object_type(object_type_a: ObjectType, object_type_b: ObjectType) -> ObjectType { + let field_map_a: IndexMap = object_type_a + .value + .fields + .into_iter() + .map_into::() + .map(|o| (o.name.to_owned(), o)) + .collect(); + let field_map_b: IndexMap = object_type_b + .value + .fields + .into_iter() + .map_into::() + .map(|o| (o.name.to_owned(), o)) + .collect(); - WithName::named( - object_type_a.name, - schema::ObjectType { - fields: merged_field_map - .into_values() - .map(WithName::into_name_value_pair) - .collect(), - description: object_type_a - .value - .description - .or(object_type_b.value.description), - }, - ) + let merged_field_map = align( + field_map_a, + field_map_b, + make_nullable_field, + make_nullable_field, + unify_object_field, + ); - } + WithName::named( + object_type_a.name, + schema::ObjectType { + fields: merged_field_map + .into_values() + .map(WithName::into_name_value_pair) + .collect(), + description: object_type_a + .value + .description + .or(object_type_b.value.description), + }, + ) } /// Unify the types of two `ObjectField`s. /// If the types are not unifiable then return an error. -fn unify_object_field(all_schema_nullable: bool) -> impl Fn(ObjectField, ObjectField) -> ObjectField { - move |object_field_a, object_field_b| - WithName::named( - object_field_a.name, - schema::ObjectField { - r#type: unify_type(object_field_a.value.r#type, object_field_b.value.r#type, all_schema_nullable), - description: object_field_a - .value - .description - .or(object_field_b.value.description), - }, - ) +fn unify_object_field(object_field_a: ObjectField, object_field_b: ObjectField) -> ObjectField { + WithName::named( + object_field_a.name, + schema::ObjectField { + r#type: unify_type(object_field_a.value.r#type, object_field_b.value.r#type), + description: object_field_a + .value + .description + .or(object_field_b.value.description), + }, + ) } /// Unify two sets of `ObjectType`s. @@ -158,7 +148,6 @@ fn unify_object_field(all_schema_nullable: bool) -> impl Fn(ObjectField, ObjectF pub fn unify_object_types( object_types_a: Vec, object_types_b: Vec, - all_schema_nullable: bool, ) -> Vec { let type_map_a: IndexMap = object_types_a .into_iter() @@ -174,7 +163,7 @@ pub fn unify_object_types( type_map_b, std::convert::identity, std::convert::identity, - unify_object_type(all_schema_nullable), + unify_object_type, ); merged_type_map.into_values().collect() @@ -199,7 +188,6 @@ mod tests { let actual = unify_type( Type::Scalar(BsonScalarType::Int), Type::Scalar(BsonScalarType::Int), - false ); assert_eq!(expected, actual); Ok(()) @@ -211,7 +199,6 @@ mod tests { let actual = unify_type( Type::Scalar(BsonScalarType::Int), Type::Scalar(BsonScalarType::String), - false ); assert_eq!(expected, actual); Ok(()) @@ -227,7 +214,7 @@ mod tests { proptest! { #[test] fn test_type_unifies_with_itself_and_normalizes(t in arb_type()) { - let u = unify_type(t.clone(), t.clone(), false); + let u = unify_type(t.clone(), t.clone()); prop_assert_eq!(t.normalize_type(), u) } } @@ -235,8 +222,8 @@ mod tests { proptest! { #[test] fn test_unify_type_is_commutative(ta in arb_type(), tb in arb_type()) { - let result_a_b = unify_type(ta.clone(), tb.clone(), false); - let result_b_a = unify_type(tb, ta, false); + let result_a_b = unify_type(ta.clone(), tb.clone()); + let result_b_a = unify_type(tb, ta); prop_assert_eq!(result_a_b, result_b_a) } } @@ -244,8 +231,8 @@ mod tests { proptest! { #[test] fn test_unify_type_is_associative(ta in arb_type(), tb in arb_type(), tc in arb_type()) { - let result_lr = unify_type(unify_type(ta.clone(), tb.clone(), false), tc.clone(), false); - let result_rl = unify_type(ta, unify_type(tb, tc, false), false); + let result_lr = unify_type(unify_type(ta.clone(), tb.clone()), tc.clone()); + let result_rl = unify_type(ta, unify_type(tb, tc)); prop_assert_eq!(result_lr, result_rl) } } @@ -253,7 +240,7 @@ mod tests { proptest! { #[test] fn test_undefined_is_left_identity(t in arb_type()) { - let u = unify_type(Type::Scalar(BsonScalarType::Undefined), t.clone(), false); + let u = unify_type(Type::Scalar(BsonScalarType::Undefined), t.clone()); prop_assert_eq!(t.normalize_type(), u) } } @@ -261,7 +248,7 @@ mod tests { proptest! { #[test] fn test_undefined_is_right_identity(t in arb_type()) { - let u = unify_type(t.clone(), Type::Scalar(BsonScalarType::Undefined), false); + let u = unify_type(t.clone(), Type::Scalar(BsonScalarType::Undefined)); prop_assert_eq!(t.normalize_type(), u) } } @@ -269,7 +256,7 @@ mod tests { proptest! { #[test] fn test_any_left(t in arb_type()) { - let u = unify_type(Type::ExtendedJSON, t, false); + let u = unify_type(Type::ExtendedJSON, t); prop_assert_eq!(Type::ExtendedJSON, u) } } @@ -277,7 +264,7 @@ mod tests { proptest! { #[test] fn test_any_right(t in arb_type()) { - let u = unify_type(t, Type::ExtendedJSON, false); + let u = unify_type(t, Type::ExtendedJSON); prop_assert_eq!(Type::ExtendedJSON, u) } } @@ -305,7 +292,7 @@ mod tests { fields: right_fields.into_iter().map(|(k, v)| (k, schema::ObjectField{r#type: v, description: None})).collect(), description: None }); - let result = unify_object_type(false)(left_object, right_object); + let result = unify_object_type(left_object, right_object); for field in result.value.named_fields() { // Any fields not shared between the two input types should be nullable. diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index c16361ba..f171e515 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -44,19 +44,19 @@ 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<()> { - let config_file = configuration::parse_configuration_options_file(&context.path).await; + 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 => config_file.sample_size + None => configuration_options.introspection_options.sample_size }; let no_validator_schema = match args.no_validator_schema { Some(validator) => validator, - None => config_file.no_validator_schema + None => configuration_options.introspection_options.no_validator_schema }; let all_schema_nullable = match args.all_schema_nullable { Some(validator) => validator, - None => config_file.all_schema_nullable + None => configuration_options.introspection_options.all_schema_nullable }; let config_file_changed = configuration::get_config_file_changed(&context.path).await?; diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs index 97cd3ce9..0579340c 100644 --- a/crates/configuration/src/configuration.rs +++ b/crates/configuration/src/configuration.rs @@ -12,28 +12,6 @@ use crate::{ read_directory, schema, serialized, }; -#[derive(Copy, Clone, Debug, Deserialize, Serialize)] -pub struct ConfigurationOptions { - // 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 ConfigurationOptions { - fn default() -> Self { - ConfigurationOptions { - sample_size: 100, - no_validator_schema: false, - all_schema_nullable: true, - } - } -} - #[derive(Clone, Debug, Default)] pub struct Configuration { /// Tracked collections from the configured MongoDB database. This includes real collections as @@ -194,6 +172,34 @@ impl Configuration { } } +#[derive(Copy, Clone, Debug, Default, Deserialize, Serialize)] +pub struct ConfigurationOptions { + // Options for introspection + pub introspection_options: ConfigurationIntrospectionOptions, +} + +#[derive(Copy, Clone, Debug, Deserialize, Serialize)] +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, From 364ebcd1bce28a466b25a9838a14cc4ef0d13ec7 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 1 May 2024 07:20:27 -0600 Subject: [PATCH 06/11] Remove wrongly commited config files --- sample_mflix_config/.configuration_metadata | 0 sample_mflix_config/configuration.json | 7 - sample_mflix_config/schema/comments.json | 44 --- .../schema/embedded_movies.json | 317 ----------------- sample_mflix_config/schema/movies.json | 318 ------------------ sample_mflix_config/schema/sessions.json | 29 -- sample_mflix_config/schema/theaters.json | 90 ----- sample_mflix_config/schema/users.json | 44 --- 8 files changed, 849 deletions(-) delete mode 100644 sample_mflix_config/.configuration_metadata delete mode 100644 sample_mflix_config/configuration.json delete mode 100644 sample_mflix_config/schema/comments.json delete mode 100644 sample_mflix_config/schema/embedded_movies.json delete mode 100644 sample_mflix_config/schema/movies.json delete mode 100644 sample_mflix_config/schema/sessions.json delete mode 100644 sample_mflix_config/schema/theaters.json delete mode 100644 sample_mflix_config/schema/users.json diff --git a/sample_mflix_config/.configuration_metadata b/sample_mflix_config/.configuration_metadata deleted file mode 100644 index e69de29b..00000000 diff --git a/sample_mflix_config/configuration.json b/sample_mflix_config/configuration.json deleted file mode 100644 index 34030b3c..00000000 --- a/sample_mflix_config/configuration.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "introspection_options": { - "sample_size": 250, - "no_validator_schema": true, - "all_schema_nullable": false - } -} \ No newline at end of file diff --git a/sample_mflix_config/schema/comments.json b/sample_mflix_config/schema/comments.json deleted file mode 100644 index bc8d022e..00000000 --- a/sample_mflix_config/schema/comments.json +++ /dev/null @@ -1,44 +0,0 @@ -{ - "name": "comments", - "collections": { - "comments": { - "type": "comments" - } - }, - "objectTypes": { - "comments": { - "fields": { - "_id": { - "type": { - "scalar": "objectId" - } - }, - "date": { - "type": { - "scalar": "date" - } - }, - "email": { - "type": { - "scalar": "string" - } - }, - "movie_id": { - "type": { - "scalar": "objectId" - } - }, - "name": { - "type": { - "scalar": "string" - } - }, - "text": { - "type": { - "scalar": "string" - } - } - } - } - } -} \ No newline at end of file diff --git a/sample_mflix_config/schema/embedded_movies.json b/sample_mflix_config/schema/embedded_movies.json deleted file mode 100644 index 281249ce..00000000 --- a/sample_mflix_config/schema/embedded_movies.json +++ /dev/null @@ -1,317 +0,0 @@ -{ - "name": "embedded_movies", - "collections": { - "embedded_movies": { - "type": "embedded_movies" - } - }, - "objectTypes": { - "embedded_movies": { - "fields": { - "_id": { - "type": { - "scalar": "objectId" - } - }, - "awards": { - "type": { - "object": "embedded_movies_awards" - } - }, - "cast": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "countries": { - "type": { - "arrayOf": { - "scalar": "string" - } - } - }, - "directors": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "fullplot": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "genres": { - "type": { - "arrayOf": { - "scalar": "string" - } - } - }, - "imdb": { - "type": { - "object": "embedded_movies_imdb" - } - }, - "languages": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "lastupdated": { - "type": { - "scalar": "string" - } - }, - "metacritic": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "num_mflix_comments": { - "type": { - "scalar": "int" - } - }, - "plot": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "plot_embedding": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "double" - } - } - } - }, - "poster": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "rated": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "released": { - "type": { - "nullable": { - "scalar": "date" - } - } - }, - "runtime": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "title": { - "type": { - "scalar": "string" - } - }, - "tomatoes": { - "type": { - "nullable": { - "object": "embedded_movies_tomatoes" - } - } - }, - "type": { - "type": { - "scalar": "string" - } - }, - "writers": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "year": { - "type": { - "scalar": "int" - } - } - } - }, - "embedded_movies_awards": { - "fields": { - "nominations": { - "type": { - "scalar": "int" - } - }, - "text": { - "type": { - "scalar": "string" - } - }, - "wins": { - "type": { - "scalar": "int" - } - } - } - }, - "embedded_movies_imdb": { - "fields": { - "id": { - "type": { - "scalar": "int" - } - }, - "rating": { - "type": { - "scalar": "double" - } - }, - "votes": { - "type": { - "scalar": "int" - } - } - } - }, - "embedded_movies_tomatoes": { - "fields": { - "boxOffice": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "consensus": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "critic": { - "type": { - "nullable": { - "object": "embedded_movies_tomatoes_critic" - } - } - }, - "dvd": { - "type": { - "nullable": { - "scalar": "date" - } - } - }, - "fresh": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "lastUpdated": { - "type": { - "scalar": "date" - } - }, - "production": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "rotten": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "viewer": { - "type": { - "object": "embedded_movies_tomatoes_viewer" - } - }, - "website": { - "type": { - "nullable": { - "scalar": "string" - } - } - } - } - }, - "embedded_movies_tomatoes_critic": { - "fields": { - "meter": { - "type": { - "scalar": "int" - } - }, - "numReviews": { - "type": { - "scalar": "int" - } - }, - "rating": { - "type": { - "scalar": "double" - } - } - } - }, - "embedded_movies_tomatoes_viewer": { - "fields": { - "meter": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "numReviews": { - "type": { - "scalar": "int" - } - }, - "rating": { - "type": { - "scalar": "double" - } - } - } - } - } -} \ No newline at end of file diff --git a/sample_mflix_config/schema/movies.json b/sample_mflix_config/schema/movies.json deleted file mode 100644 index 97817783..00000000 --- a/sample_mflix_config/schema/movies.json +++ /dev/null @@ -1,318 +0,0 @@ -{ - "name": "movies", - "collections": { - "movies": { - "type": "movies" - } - }, - "objectTypes": { - "movies": { - "fields": { - "_id": { - "type": { - "scalar": "objectId" - } - }, - "awards": { - "type": { - "object": "movies_awards" - } - }, - "cast": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "countries": { - "type": { - "arrayOf": { - "scalar": "string" - } - } - }, - "directors": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "fullplot": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "genres": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "imdb": { - "type": { - "object": "movies_imdb" - } - }, - "languages": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "lastupdated": { - "type": { - "scalar": "string" - } - }, - "metacritic": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "num_mflix_comments": { - "type": { - "scalar": "int" - } - }, - "plot": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "poster": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "rated": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "released": { - "type": { - "nullable": { - "scalar": "date" - } - } - }, - "runtime": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "title": { - "type": { - "scalar": "string" - } - }, - "tomatoes": { - "type": { - "nullable": { - "object": "movies_tomatoes" - } - } - }, - "type": { - "type": { - "scalar": "string" - } - }, - "writers": { - "type": { - "nullable": { - "arrayOf": { - "scalar": "string" - } - } - } - }, - "year": { - "type": { - "scalar": "int" - } - } - } - }, - "movies_awards": { - "fields": { - "nominations": { - "type": { - "scalar": "int" - } - }, - "text": { - "type": { - "scalar": "string" - } - }, - "wins": { - "type": { - "scalar": "int" - } - } - } - }, - "movies_imdb": { - "fields": { - "id": { - "type": { - "scalar": "int" - } - }, - "rating": { - "type": { - "scalar": "double" - } - }, - "votes": { - "type": { - "scalar": "int" - } - } - } - }, - "movies_tomatoes": { - "fields": { - "boxOffice": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "consensus": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "critic": { - "type": { - "nullable": { - "object": "movies_tomatoes_critic" - } - } - }, - "dvd": { - "type": { - "nullable": { - "scalar": "date" - } - } - }, - "fresh": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "lastUpdated": { - "type": { - "scalar": "date" - } - }, - "production": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "rotten": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "viewer": { - "type": { - "object": "movies_tomatoes_viewer" - } - }, - "website": { - "type": { - "nullable": { - "scalar": "string" - } - } - } - } - }, - "movies_tomatoes_critic": { - "fields": { - "meter": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "numReviews": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "rating": { - "type": { - "nullable": { - "scalar": "double" - } - } - } - } - }, - "movies_tomatoes_viewer": { - "fields": { - "meter": { - "type": { - "nullable": { - "scalar": "int" - } - } - }, - "numReviews": { - "type": { - "scalar": "int" - } - }, - "rating": { - "type": { - "nullable": { - "scalar": "double" - } - } - } - } - } - } -} \ No newline at end of file diff --git a/sample_mflix_config/schema/sessions.json b/sample_mflix_config/schema/sessions.json deleted file mode 100644 index 364b9050..00000000 --- a/sample_mflix_config/schema/sessions.json +++ /dev/null @@ -1,29 +0,0 @@ -{ - "name": "sessions", - "collections": { - "sessions": { - "type": "sessions" - } - }, - "objectTypes": { - "sessions": { - "fields": { - "_id": { - "type": { - "scalar": "objectId" - } - }, - "jwt": { - "type": { - "scalar": "string" - } - }, - "user_id": { - "type": { - "scalar": "string" - } - } - } - } - } -} \ No newline at end of file diff --git a/sample_mflix_config/schema/theaters.json b/sample_mflix_config/schema/theaters.json deleted file mode 100644 index df44678b..00000000 --- a/sample_mflix_config/schema/theaters.json +++ /dev/null @@ -1,90 +0,0 @@ -{ - "name": "theaters", - "collections": { - "theaters": { - "type": "theaters" - } - }, - "objectTypes": { - "theaters": { - "fields": { - "_id": { - "type": { - "scalar": "objectId" - } - }, - "location": { - "type": { - "object": "theaters_location" - } - }, - "theaterId": { - "type": { - "scalar": "int" - } - } - } - }, - "theaters_location": { - "fields": { - "address": { - "type": { - "object": "theaters_location_address" - } - }, - "geo": { - "type": { - "object": "theaters_location_geo" - } - } - } - }, - "theaters_location_address": { - "fields": { - "city": { - "type": { - "scalar": "string" - } - }, - "state": { - "type": { - "scalar": "string" - } - }, - "street1": { - "type": { - "scalar": "string" - } - }, - "street2": { - "type": { - "nullable": { - "scalar": "string" - } - } - }, - "zipcode": { - "type": { - "scalar": "string" - } - } - } - }, - "theaters_location_geo": { - "fields": { - "coordinates": { - "type": { - "arrayOf": { - "scalar": "double" - } - } - }, - "type": { - "type": { - "scalar": "string" - } - } - } - } - } -} \ No newline at end of file diff --git a/sample_mflix_config/schema/users.json b/sample_mflix_config/schema/users.json deleted file mode 100644 index ec2b7149..00000000 --- a/sample_mflix_config/schema/users.json +++ /dev/null @@ -1,44 +0,0 @@ -{ - "name": "users", - "collections": { - "users": { - "type": "users" - } - }, - "objectTypes": { - "users": { - "fields": { - "_id": { - "type": { - "scalar": "objectId" - } - }, - "email": { - "type": { - "scalar": "string" - } - }, - "name": { - "type": { - "scalar": "string" - } - }, - "password": { - "type": { - "scalar": "string" - } - }, - "preferences": { - "type": { - "nullable": { - "object": "users_preferences" - } - } - } - } - }, - "users_preferences": { - "fields": {} - } - } -} \ No newline at end of file From 9a7829b065d1e9209022d29f9bc58f3631e69a1c Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 1 May 2024 07:22:04 -0600 Subject: [PATCH 07/11] Add comment --- crates/configuration/src/directory.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index ff7dde5c..32fb6193 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -213,6 +213,8 @@ pub async fn list_existing_schemas( Ok(schemas.into_keys().collect()) } +// Metadata file is just a dot filed used for the purposes of know if the user has updated their config to force refresh +// of the schema introspection. async fn write_config_metadata_file( configuration_dir: impl AsRef ) -> () { From ff0c72ebfe1912823e3c19025420317c022f43d5 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 1 May 2024 07:24:04 -0600 Subject: [PATCH 08/11] Fix bad rebase --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a8bdbb9..13b653e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ 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 an optional root `configuration.json` or `configuration.yaml` file to allow editing cli options. Update default sample size to 100. ([#68](https://github.com/hasura/ndc-mongodb/pull/68)) - 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)) From e98c77d4847a962dbcf24fcb6ec8c02350d46e6b Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 1 May 2024 08:57:10 -0600 Subject: [PATCH 09/11] Fix bad revert from previous commits --- crates/cli/src/introspection/type_unification.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cli/src/introspection/type_unification.rs b/crates/cli/src/introspection/type_unification.rs index 54727fe8..61a8a377 100644 --- a/crates/cli/src/introspection/type_unification.rs +++ b/crates/cli/src/introspection/type_unification.rs @@ -73,7 +73,7 @@ pub fn unify_type(type_a: Type, type_b: Type) -> Type { (_, _) => Type::ExtendedJSON, }; - result_type + result_type.normalize_type() } pub fn make_nullable_field(field: ObjectField) -> ObjectField { From 3c9832b36ef6d4dfc18d7d37566011e7941e128d Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 1 May 2024 09:05:39 -0600 Subject: [PATCH 10/11] Lint fix --- crates/configuration/src/directory.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index 32fb6193..369d7357 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -225,11 +225,9 @@ async fn write_config_metadata_file( .create(true) .open(dir.join(CONFIGURATION_OPTIONS_METADATA)) .await; - match file_result { - Ok(mut file) => { - let _ = file.write_all(b"").await; - }, - Err(_) => () + + if let Ok(mut file) = file_result { + let _ = file.write_all(b"").await; }; } From ca52982ed5fbe22bb1309f6eefe114c42c2cb5c0 Mon Sep 17 00:00:00 2001 From: Brandon Martin Date: Wed, 1 May 2024 10:54:15 -0600 Subject: [PATCH 11/11] Review feedback --- crates/configuration/src/configuration.rs | 2 ++ crates/configuration/src/directory.rs | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs index 0579340c..b5a78bc3 100644 --- a/crates/configuration/src/configuration.rs +++ b/crates/configuration/src/configuration.rs @@ -173,12 +173,14 @@ 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, diff --git a/crates/configuration/src/directory.rs b/crates/configuration/src/directory.rs index 369d7357..b66eee8d 100644 --- a/crates/configuration/src/directory.rs +++ b/crates/configuration/src/directory.rs @@ -48,7 +48,7 @@ pub async fn read_directory( .await? .unwrap_or_default(); - let options = parse_configuration_options_file(&dir) + let options = parse_configuration_options_file(dir) .await; Configuration::validate(schema, native_procedures, native_queries, options) @@ -129,7 +129,7 @@ pub async fn parse_configuration_options_file(dir: &Path) -> ConfigurationOption let defaults: ConfigurationOptions = Default::default(); let _ = write_file(dir, CONFIGURATION_OPTIONS_BASENAME, &defaults).await; let _ = write_config_metadata_file(dir).await; - return defaults + defaults } async fn parse_config_file(path: impl AsRef, format: FileFormat) -> anyhow::Result @@ -217,7 +217,7 @@ pub async fn list_existing_schemas( // of the schema introspection. async fn write_config_metadata_file( configuration_dir: impl AsRef -) -> () { +) { let dir = configuration_dir.as_ref(); let file_result = fs::OpenOptions::new() .write(true)