From 076ec88c13b8a08b6814a093ccb713faad512d83 Mon Sep 17 00:00:00 2001 From: Gil Mizrahi Date: Thu, 14 Mar 2024 11:24:58 +0200 Subject: [PATCH] update command should not override the config file in some cases --- crates/cli/src/lib.rs | 87 +++++++++++++------ crates/configuration/src/configuration.rs | 2 +- .../src/values/isolation_level.rs | 2 +- .../configuration/src/values/pool_settings.rs | 2 +- .../configuration/src/version3/comparison.rs | 2 +- .../src/version3/connection_settings.rs | 2 +- crates/configuration/src/version3/mod.rs | 2 +- crates/configuration/src/version3/options.rs | 2 +- .../metadata/src/metadata/database.rs | 4 +- .../query-engine/metadata/src/metadata/mod.rs | 2 +- .../metadata/src/metadata/mutations.rs | 2 +- .../metadata/src/metadata/native_queries.rs | 2 +- 12 files changed, 74 insertions(+), 37 deletions(-) diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 8559686a4..417b164de 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -13,6 +13,8 @@ use tokio::fs; use ndc_postgres_configuration as configuration; use ndc_postgres_configuration::environment::Environment; +const UPDATE_ATTEMPTS: u8 = 3; + /// The various contextual bits and bobs we need to run. pub struct Context { pub context_path: PathBuf, @@ -128,30 +130,65 @@ async fn initialize(with_metadata: bool, context: Context) -> /// /// This expects a configuration with a valid connection URI. async fn update(context: Context) -> anyhow::Result<()> { - let configuration_file_path = context - .context_path - .join(configuration::CONFIGURATION_FILENAME); - let input: configuration::RawConfiguration = { - let configuration_file_contents = fs::read_to_string(&configuration_file_path) - .await - .map_err(|err| { - if err.kind() == std::io::ErrorKind::NotFound { - anyhow::anyhow!( - "{}: No such file or directory. Perhaps you meant to 'initialize' first?", - configuration_file_path.display() - ) - } else { - anyhow::anyhow!(err) - } - })?; - serde_json::from_str(&configuration_file_contents)? - }; - let output = configuration::introspect(input, &context.environment).await?; + // It is possible to change the file in the middle of introspection. + // We want to detect these scenario and try again, or fail if we are unable to. + // We do that with a few attempts. + for _attempt in 1..UPDATE_ATTEMPTS { + let configuration_file_path = context + .context_path + .join(configuration::CONFIGURATION_FILENAME); + let input: configuration::RawConfiguration = { + let configuration_file_contents = + read_config_file_contents(&configuration_file_path).await?; + serde_json::from_str(&configuration_file_contents)? + }; + let output = configuration::introspect(input.clone(), &context.environment).await?; - fs::write( - &configuration_file_path, - serde_json::to_string_pretty(&output)?, - ) - .await?; - Ok(()) + // Check that the input file did not change since we started introspecting. + let input_again_before_write: configuration::RawConfiguration = { + let configuration_file_contents = + read_config_file_contents(&configuration_file_path).await?; + serde_json::from_str(&configuration_file_contents)? + }; + + if input_again_before_write != input { + println!("Input file changed before write, trying again."); + // next attempt + continue; + } + + // If the introspection result is different than the current config, + // change it. Otherwise, continue. + if input != output { + fs::write( + &configuration_file_path, + serde_json::to_string_pretty(&output)?, + ) + .await?; + } else { + println!("The configuration is up-to-date. Nothing to do."); + } + + return Ok(()); + } + + // We ran out of attempts. + Err(anyhow::anyhow!( + "Cannot override configuration: input changed before write." + )) +} + +async fn read_config_file_contents(configuration_file_path: &PathBuf) -> anyhow::Result { + fs::read_to_string(configuration_file_path) + .await + .map_err(|err| { + if err.kind() == std::io::ErrorKind::NotFound { + anyhow::anyhow!( + "{}: No such file or directory. Perhaps you meant to 'initialize' first?", + configuration_file_path.display() + ) + } else { + anyhow::anyhow!(err) + } + }) } diff --git a/crates/configuration/src/configuration.rs b/crates/configuration/src/configuration.rs index 4e29d9e44..4d55ae26c 100644 --- a/crates/configuration/src/configuration.rs +++ b/crates/configuration/src/configuration.rs @@ -17,7 +17,7 @@ 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)] +#[derive(Clone, PartialEq, Eq, Debug, Deserialize, Serialize, JsonSchema)] #[serde(tag = "version")] pub enum RawConfiguration { #[serde(rename = "3")] diff --git a/crates/configuration/src/values/isolation_level.rs b/crates/configuration/src/values/isolation_level.rs index 2147ddc9b..9dcddc2b0 100644 --- a/crates/configuration/src/values/isolation_level.rs +++ b/crates/configuration/src/values/isolation_level.rs @@ -2,7 +2,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; /// The isolation level of the transaction in which a query is executed. -#[derive(Debug, Clone, Copy, Default, Deserialize, Serialize, JsonSchema)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Default, Deserialize, Serialize, JsonSchema)] pub enum IsolationLevel { /// Prevents reading data from another uncommitted transaction. #[default] diff --git a/crates/configuration/src/values/pool_settings.rs b/crates/configuration/src/values/pool_settings.rs index 273c1171e..c2ad17653 100644 --- a/crates/configuration/src/values/pool_settings.rs +++ b/crates/configuration/src/values/pool_settings.rs @@ -2,7 +2,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; /// Settings for the PostgreSQL connection pool -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, JsonSchema)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct PoolSettings { /// maximum number of pool connections diff --git a/crates/configuration/src/version3/comparison.rs b/crates/configuration/src/version3/comparison.rs index 99598fa8c..73c72c627 100644 --- a/crates/configuration/src/version3/comparison.rs +++ b/crates/configuration/src/version3/comparison.rs @@ -5,7 +5,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; /// Define the names that comparison operators will be exposed as by the automatic introspection. -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct ComparisonOperatorMapping { /// The name of the operator as defined by the database diff --git a/crates/configuration/src/version3/connection_settings.rs b/crates/configuration/src/version3/connection_settings.rs index df6d0bb18..4ecd3662c 100644 --- a/crates/configuration/src/version3/connection_settings.rs +++ b/crates/configuration/src/version3/connection_settings.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; pub const DEFAULT_CONNECTION_URI_VARIABLE: &str = "CONNECTION_URI"; /// Database connection settings. -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[derive(Clone, PartialEq, Eq, Debug, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct DatabaseConnectionSettings { /// Connection string for a Postgres-compatible database. diff --git a/crates/configuration/src/version3/mod.rs b/crates/configuration/src/version3/mod.rs index b64c19497..8c661b1cb 100644 --- a/crates/configuration/src/version3/mod.rs +++ b/crates/configuration/src/version3/mod.rs @@ -24,7 +24,7 @@ const CONFIGURATION_QUERY: &str = include_str!("version3.sql"); /// Initial configuration, just enough to connect to a database and elaborate a full /// 'Configuration'. -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[derive(Clone, PartialEq, Eq, Debug, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct RawConfiguration { /// Jsonschema of the configuration format. diff --git a/crates/configuration/src/version3/options.rs b/crates/configuration/src/version3/options.rs index e95d09049..794147180 100644 --- a/crates/configuration/src/version3/options.rs +++ b/crates/configuration/src/version3/options.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use super::comparison::ComparisonOperatorMapping; /// Options which only influence how the configuration is updated. -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[derive(Clone, PartialEq, Eq, Debug, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct IntrospectionOptions { /// Schemas which are excluded from introspection. The default setting will exclude the diff --git a/crates/query-engine/metadata/src/metadata/database.rs b/crates/query-engine/metadata/src/metadata/database.rs index c35368774..9f084835b 100644 --- a/crates/query-engine/metadata/src/metadata/database.rs +++ b/crates/query-engine/metadata/src/metadata/database.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, BTreeSet}; /// Map of all known composite types. -#[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct CompositeTypes(pub BTreeMap); @@ -79,7 +79,7 @@ fn default_true() -> bool { } /// Mapping from a "table" name to its information. -#[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct TablesInfo(pub BTreeMap); diff --git a/crates/query-engine/metadata/src/metadata/mod.rs b/crates/query-engine/metadata/src/metadata/mod.rs index 83692b8a8..5d96146dc 100644 --- a/crates/query-engine/metadata/src/metadata/mod.rs +++ b/crates/query-engine/metadata/src/metadata/mod.rs @@ -12,7 +12,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; /// Metadata information. -#[derive(Clone, Debug, Default, Serialize, Deserialize, JsonSchema)] +#[derive(Clone, PartialEq, Eq, Debug, Default, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct Metadata { #[serde(default)] diff --git a/crates/query-engine/metadata/src/metadata/mutations.rs b/crates/query-engine/metadata/src/metadata/mutations.rs index 34ff9256d..482ba641d 100644 --- a/crates/query-engine/metadata/src/metadata/mutations.rs +++ b/crates/query-engine/metadata/src/metadata/mutations.rs @@ -4,7 +4,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; /// Which version of the generated mutations will be included in the schema -#[derive(Debug, Clone, Copy, Deserialize, Serialize, JsonSchema)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub enum MutationsVersion { V1, diff --git a/crates/query-engine/metadata/src/metadata/native_queries.rs b/crates/query-engine/metadata/src/metadata/native_queries.rs index 2c53fd4e2..54d21836d 100644 --- a/crates/query-engine/metadata/src/metadata/native_queries.rs +++ b/crates/query-engine/metadata/src/metadata/native_queries.rs @@ -9,7 +9,7 @@ use std::collections::BTreeMap; // Types /// Metadata information of native queries. -#[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct NativeQueries(pub BTreeMap);