Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename schema fields and add a jsonschema generation and ref on init #361

Merged
merged 9 commits into from
Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@

## [Unreleased]

### Changed

- The ndc-postgres-cli `initialize` command will now generate
the jsonschema of the configuration format as well.
([#361](https://github.com/hasura/ndc-postgres/pull/361))
- A few fields in the configuration format has been changed:

- `configureOptions` was renamed to `introspectionOptions`
- `connectionUri`, `poolSettings` and `isolationLevel` are now nested under `connectionSettings`
- `mutationsVersion` was moved from `configureOptions` to the top-level
- A new field was added: `$schema` references the jsonschema of the configuration format

([#361](https://github.com/hasura/ndc-postgres/pull/361))

## [v0.4.1] - 2024-03-06

### Fixed
Expand Down
1 change: 1 addition & 0 deletions crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ ndc-postgres-configuration = { path = "../configuration" }

anyhow = "1.0.80"
clap = { version = "4.5.2", features = ["derive", "env"] }
schemars = { version = "0.8.16", features = ["smol_str", "preserve_order"] }
serde = { version = "1.0.197", features = ["derive"] }
serde_json = { version = "1.0.114", features = ["raw_value"] }
serde_yaml = "0.9.32"
Expand Down
14 changes: 14 additions & 0 deletions crates/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub enum Command {
/// Initialize a configuration in the current (empty) directory.
Initialize {
#[arg(long)]
/// Whether to create the hasura connector metadata.
with_metadata: bool,
},
/// Update the configuration by introspecting the database, using the configuration options.
Expand Down Expand Up @@ -75,6 +76,18 @@ async fn initialize(with_metadata: bool, context: Context<impl Environment>) ->
)
.await?;

// create the jsonschema file
let configuration_jsonschema_file_path = context
.context_path
.join(configuration::CONFIGURATION_JSONSCHEMA_FILENAME);

let output = schemars::schema_for!(ndc_postgres_configuration::RawConfiguration);
fs::write(
&configuration_jsonschema_file_path,
serde_json::to_string_pretty(&output)?,
)
.await?;

// if requested, create the metadata
if with_metadata {
let metadata_dir = context.context_path.join(".hasura-connector");
Expand Down Expand Up @@ -134,6 +147,7 @@ async fn update(context: Context<impl Environment>) -> anyhow::Result<()> {
serde_json::from_str(&configuration_file_contents)?
};
let output = configuration::introspect(input, &context.environment).await?;

fs::write(
&configuration_file_path,
serde_json::to_string_pretty(&output)?,
Expand Down
11 changes: 8 additions & 3 deletions crates/cli/tests/update_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@ async fn test_update_configuration() -> anyhow::Result<()> {

{
let configuration_file_path = dir.path().join("configuration.json");
let connection_settings =
configuration::version3::connection_settings::DatabaseConnectionSettings {
connection_uri: connection_uri.clone(),
..configuration::version3::connection_settings::DatabaseConnectionSettings::empty()
};
let input = RawConfiguration::Version3(configuration::version3::RawConfiguration {
connection_uri: connection_uri.clone(),
connection_settings,
..configuration::version3::RawConfiguration::empty()
});
fs::write(configuration_file_path, serde_json::to_string(&input)?).await?;
Expand All @@ -39,11 +44,11 @@ async fn test_update_configuration() -> anyhow::Result<()> {
let output: RawConfiguration = serde_json::from_str(&contents)?;
match output {
RawConfiguration::Version3(configuration::version3::RawConfiguration {
connection_uri: updated_connection_uri,
connection_settings,
metadata,
..
}) => {
assert_eq!(updated_connection_uri, connection_uri);
assert_eq!(connection_settings.connection_uri, connection_uri);
let some_table_metadata = metadata.tables.0.get("Artist");
assert!(some_table_metadata.is_some());
}
Expand Down
9 changes: 5 additions & 4 deletions crates/configuration/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::values::{ConnectionUri, IsolationLevel, PoolSettings, Secret};
use crate::version3;

pub const CONFIGURATION_FILENAME: &str = "configuration.json";
pub const CONFIGURATION_JSONSCHEMA_FILENAME: &str = "schema.json";

/// The parsed connector configuration.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
Expand Down Expand Up @@ -68,7 +69,7 @@ pub async fn parse_configuration(
message: error.to_string(),
})?;
let connection_uri =
match configuration.connection_uri {
match configuration.connection_settings.connection_uri {
ConnectionUri(Secret::Plain(uri)) => Ok(uri),
ConnectionUri(Secret::FromEnvironment { variable }) => environment
.read(&variable)
Expand All @@ -79,9 +80,9 @@ pub async fn parse_configuration(
}?;
Ok(Configuration {
metadata: configuration.metadata,
pool_settings: configuration.pool_settings,
pool_settings: configuration.connection_settings.pool_settings,
connection_uri,
isolation_level: configuration.isolation_level,
mutations_version: configuration.configure_options.mutations_version,
isolation_level: configuration.connection_settings.isolation_level,
mutations_version: configuration.mutations_version,
Comment on lines -82 to +86
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it's worth just storing connection_settings directly in the Configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought about it, but didn't want to make too many changes, and wasn't sure about whether we should use version3 structure here. Maybe we can decide in a separate PR since this is internal to the connector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I do agree that that's a better structure though.

})
}
1 change: 1 addition & 0 deletions crates/configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub mod version3;

pub use configuration::{
introspect, parse_configuration, Configuration, RawConfiguration, CONFIGURATION_FILENAME,
CONFIGURATION_JSONSCHEMA_FILENAME,
};
pub use error::Error;
pub use values::{ConnectionUri, IsolationLevel, PoolSettings, Secret};
Expand Down
33 changes: 33 additions & 0 deletions crates/configuration/src/version3/connection_settings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//! Database connection settings.

use crate::values::{ConnectionUri, IsolationLevel, PoolSettings, Secret};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

pub const DEFAULT_CONNECTION_URI_VARIABLE: &str = "CONNECTION_URI";

/// Database connection settings.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct DatabaseConnectionSettings {
/// Connection string for a Postgres-compatible database.
pub connection_uri: ConnectionUri,
/// Connection pool settings.
#[serde(default)]
pub pool_settings: PoolSettings,
/// Query isolation level.
#[serde(default)]
pub isolation_level: IsolationLevel,
}

impl DatabaseConnectionSettings {
pub fn empty() -> Self {
Self {
connection_uri: ConnectionUri(Secret::FromEnvironment {
variable: DEFAULT_CONNECTION_URI_VARIABLE.into(),
}),
pool_settings: PoolSettings::default(),
isolation_level: IsolationLevel::default(),
}
}
}
74 changes: 43 additions & 31 deletions crates/configuration/src/version3/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Internal Configuration and state for our connector.

mod comparison;
pub mod connection_settings;
mod options;

use std::borrow::Cow;
Expand All @@ -17,39 +18,42 @@ use query_engine_metadata::metadata;

use crate::environment::Environment;
use crate::error::Error;
use crate::values::{ConnectionUri, IsolationLevel, PoolSettings, Secret};
use crate::values::{ConnectionUri, Secret};

const CONFIGURATION_QUERY: &str = include_str!("version3.sql");

pub const DEFAULT_CONNECTION_URI_VARIABLE: &str = "CONNECTION_URI";

/// Initial configuration, just enough to connect to a database and elaborate a full
/// 'Configuration'.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct RawConfiguration {
// Connection string for a Postgres-compatible database
pub connection_uri: ConnectionUri,
#[serde(default)]
pub pool_settings: PoolSettings,
/// Jsonschema of the configuration format.
#[serde(rename = "$schema")]
#[serde(default)]
pub isolation_level: IsolationLevel,
pub schema: Option<String>,
/// Database connection settings.
pub connection_settings: connection_settings::DatabaseConnectionSettings,
/// Connector metadata.
#[serde(default)]
pub metadata: metadata::Metadata,
/// Database introspection options.
#[serde(default)]
pub introspection_options: options::IntrospectionOptions,
/// Which version of the generated mutation procedures to include in the schema response
#[serde(default)]
pub configure_options: options::ConfigureOptions,
pub mutations_version: Option<metadata::mutations::MutationsVersion>,
}

impl RawConfiguration {
pub fn empty() -> Self {
Self {
connection_uri: ConnectionUri(Secret::FromEnvironment {
variable: DEFAULT_CONNECTION_URI_VARIABLE.into(),
}),
pool_settings: PoolSettings::default(),
isolation_level: IsolationLevel::default(),
schema: Some(crate::CONFIGURATION_JSONSCHEMA_FILENAME.to_string()),
connection_settings: connection_settings::DatabaseConnectionSettings::empty(),
metadata: metadata::Metadata::default(),
configure_options: options::ConfigureOptions::default(),
introspection_options: options::IntrospectionOptions::default(),
// we'll change this to `Some(MutationsVersions::V1)` when we
// want to "release" this behaviour
mutations_version: None,
}
}
}
Expand All @@ -59,7 +63,7 @@ pub async fn validate_raw_configuration(
file_path: PathBuf,
config: RawConfiguration,
) -> Result<RawConfiguration, Error> {
match &config.connection_uri {
match &config.connection_settings.connection_uri {
ConnectionUri(Secret::Plain(uri)) if uri.is_empty() => {
Err(Error::EmptyConnectionUri { file_path })
}
Expand All @@ -74,7 +78,7 @@ pub async fn introspect(
args: RawConfiguration,
environment: impl Environment,
) -> anyhow::Result<RawConfiguration> {
let uri = match &args.connection_uri {
let uri = match &args.connection_settings.connection_uri {
ConnectionUri(Secret::Plain(value)) => Cow::Borrowed(value),
ConnectionUri(Secret::FromEnvironment { variable }) => {
Cow::Owned(environment.read(variable)?)
Expand All @@ -86,19 +90,19 @@ pub async fn introspect(
.await?;

let query = sqlx::query(CONFIGURATION_QUERY)
.bind(&args.configure_options.excluded_schemas)
.bind(&args.configure_options.unqualified_schemas_for_tables)
.bind(&args.introspection_options.excluded_schemas)
.bind(&args.introspection_options.unqualified_schemas_for_tables)
.bind(
&args
.configure_options
.introspection_options
.unqualified_schemas_for_types_and_procedures,
)
.bind(serde_json::to_value(
&args.configure_options.comparison_operator_mapping,
&args.introspection_options.comparison_operator_mapping,
)?)
.bind(
&args
.configure_options
.introspection_options
.introspect_prefix_function_comparison_operators,
);

Expand Down Expand Up @@ -155,17 +159,17 @@ pub async fn introspect(
filter_aggregate_functions(&scalar_types, aggregate_functions);

Ok(RawConfiguration {
connection_uri: args.connection_uri,
pool_settings: args.pool_settings,
isolation_level: args.isolation_level,
schema: args.schema,
connection_settings: args.connection_settings,
metadata: metadata::Metadata {
tables,
native_queries: args.metadata.native_queries,
aggregate_functions: relevant_aggregate_functions,
comparison_operators: relevant_comparison_operators,
composite_types: args.metadata.composite_types,
},
configure_options: args.configure_options,
introspection_options: args.introspection_options,
mutations_version: args.mutations_version,
})
}

Expand All @@ -175,15 +179,23 @@ pub fn occurring_scalar_types(
tables: &metadata::TablesInfo,
native_queries: &metadata::NativeQueries,
) -> BTreeSet<metadata::ScalarType> {
let tables_column_types = tables.0.values().flat_map(|v| v.columns.values());
let native_queries_column_types = native_queries.0.values().flat_map(|v| v.columns.values());
let native_queries_arguments_types =
native_queries.0.values().flat_map(|v| v.arguments.values());
let tables_column_types = tables
.0
.values()
.flat_map(|v| v.columns.values().map(|c| &c.r#type));
let native_queries_column_types = native_queries
.0
.values()
.flat_map(|v| v.columns.values().map(|c| &c.r#type));
let native_queries_arguments_types = native_queries
.0
.values()
.flat_map(|v| v.arguments.values().map(|c| &c.r#type));

tables_column_types
.chain(native_queries_column_types)
.chain(native_queries_arguments_types)
.filter_map(|c| match c.r#type {
.filter_map(|t| match t {
metadata::Type::ScalarType(ref t) => Some(t.clone()), // only keep scalar types
metadata::Type::ArrayType(_) | metadata::Type::CompositeType(_) => None,
})
Expand Down
16 changes: 4 additions & 12 deletions crates/configuration/src/version3/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};

use query_engine_metadata::metadata;

use super::comparison::ComparisonOperatorMapping;

/// Options which only influence how the configuration is updated.
#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct ConfigureOptions {
pub struct IntrospectionOptions {
/// Schemas which are excluded from introspection. The default setting will exclude the
/// internal schemas of Postgres, Citus, Cockroach, and the PostGIS extension.
#[serde(default = "default_excluded_schemas")]
Expand All @@ -25,9 +23,6 @@ pub struct ConfigureOptions {
/// The mapping of comparison operator names to apply when updating the configuration
#[serde(default = "ComparisonOperatorMapping::default_mappings")]
pub comparison_operator_mapping: Vec<ComparisonOperatorMapping>,
/// Which version of the generated mutation procedures to include in the schema response
#[serde(default)]
pub mutations_version: Option<metadata::mutations::MutationsVersion>,
/// Which prefix functions (i.e., non-infix operators) to generate introspection metadata for.
///
/// This list will accept any boolean-returning function taking two concrete scalar types as
Expand All @@ -38,17 +33,14 @@ pub struct ConfigureOptions {
pub introspect_prefix_function_comparison_operators: Vec<String>,
}

impl Default for ConfigureOptions {
fn default() -> ConfigureOptions {
ConfigureOptions {
impl Default for IntrospectionOptions {
fn default() -> IntrospectionOptions {
IntrospectionOptions {
excluded_schemas: default_excluded_schemas(),
unqualified_schemas_for_tables: default_unqualified_schemas_for_tables(),
unqualified_schemas_for_types_and_procedures:
default_unqualified_schemas_for_types_and_procedures(),
comparison_operator_mapping: ComparisonOperatorMapping::default_mappings(),
// we'll change this to `Some(MutationsVersions::V1)` when we
// want to "release" this behaviour
mutations_version: None,
introspect_prefix_function_comparison_operators:
default_introspect_prefix_function_comparison_operators(),
}
Expand Down
Loading
Loading