Migrate tests to access configuration through public interface

Cargo.toml

@@ -2,9 +2,7 @@
 resolver = "2"
 
 package.version = "0.6.0"
-
 package.edition = "2021"
-
 package.license = "Apache-2.0"
 
 members = [
@@ -31,5 +29,6 @@ similar_names = "allow"
 too_many_lines = "allow"
 
 [workspace.dependencies]
+ndc-models = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.2" }
 ndc-sdk = { git = "https://github.com/hasura/ndc-sdk-rs.git", rev = "6158b1adbcc4ac7d8acb721c1626f6f715424a27" }
 ndc-test = { git = "https://github.com/hasura/ndc-spec.git", tag = "v0.1.2" }

crates/cli/tests/update_tests.rs

@@ -13,28 +13,14 @@ const CONNECTION_URI: &str = "postgresql://postgres:password@localhost:64002";
 async fn test_update_configuration() -> anyhow::Result<()> {
     let dir = tempfile::tempdir()?;
 
-    let connection_uri = configuration::ConnectionUri(configuration::Secret::FromEnvironment {
-        variable: "CONNECTION_URI".into(),
-    });
-
-    {
-        let connection_settings =
-            configuration::version3::connection_settings::DatabaseConnectionSettings {
-                connection_uri: connection_uri.clone(),
-                ..configuration::version3::connection_settings::DatabaseConnectionSettings::empty()
-            };
-        let input = ParsedConfiguration::Version3(configuration::version3::RawConfiguration {
-            connection_settings,
-            ..configuration::version3::RawConfiguration::empty()
-        });
-        configuration::write_parsed_configuration(input, &dir).await?;
-    }
+    configuration::write_parsed_configuration(configuration::ParsedConfiguration::initial(), &dir)
+        .await?;
 
     let environment =
         FixedEnvironment::from([("CONNECTION_URI".into(), CONNECTION_URI.to_string())]);
     let context = Context {
         context_path: dir.path().to_owned(),
-        environment,
+        environment: environment.clone(),
         release_version: None,
     };
     run(Command::Update, context).await?;
@@ -44,18 +30,10 @@ async fn test_update_configuration() -> anyhow::Result<()> {
     let contents = fs::read_to_string(configuration_file_path).await?;
     common::assert_ends_with_newline(&contents);
     let output: ParsedConfiguration = configuration::parse_configuration(&dir).await?;
-    match output {
-        ParsedConfiguration::Version3(configuration::version3::RawConfiguration {
-            connection_settings,
-            metadata,
-            ..
-        }) => {
-            assert_eq!(connection_settings.connection_uri, connection_uri);
-            let some_table_metadata = metadata.tables.0.get("Artist");
-            assert!(some_table_metadata.is_some());
-        }
-        ParsedConfiguration::Version4(_) => panic!("Expected version 3"),
-    }
+    let runtime_config = configuration::make_runtime_configuration(output, environment)?;
+
+    let some_table_metadata = runtime_config.metadata.tables.0.get("Artist");
+    assert!(some_table_metadata.is_some());
 
     Ok(())
 }

crates/configuration/Cargo.toml

@@ -16,5 +16,10 @@ serde = "1.0.200"
 serde_json = { version = "1.0.116", features = ["raw_value"] }
 sqlx = { version = "0.7.4", features = ["json", "postgres", "runtime-tokio-rustls"] }
 thiserror = "1.0.59"
-tokio = "1.37.0"
+tokio = { version = "1.37.0", features = ["full"]}
 tracing = "0.1.40"
+
+[dev-dependencies]
+insta = { version = "1.38.0", features = ["json"]}
+jsonschema = "0.17.1"
+similar-asserts = "1.5.0"

crates/configuration/src/configuration.rs

@@ -12,17 +12,28 @@ use crate::error::{
 use crate::values::{IsolationLevel, PoolSettings};
 use crate::version3;
 use crate::version4;
+use schemars::{gen::SchemaSettings, schema::RootSchema};
 
-/// The parsed connector configuration. This data type is an enum with cases for each supported
-/// version.
+pub fn generate_latest_schema() -> RootSchema {
+    SchemaSettings::openapi3()
+        .into_generator()
+        .into_root_schema_for::<version4::ParsedConfiguration>()
+}
+
+pub const DEFAULT_CONNECTION_URI_VARIABLE: &str = "CONNECTION_URI";
+
+/// The 'ParsedConfiguration' type models the various concrete configuration formats that are
+/// currently supported.
+///
+/// Introducing a breaking configuration format change involves adding a new case to this type.
 ///
-/// It supports various uses:
+/// 'ParsedConfiguration' is used to support serialization and deserialization of an NDC
+/// configuration. It retains all the salient information that constitues an instance of an NDC
+/// deployment, such that 'c = parse_configuration(dir) => { write_parsed_configuration(c, dir2) ;
+/// assert(c == parse_configuration(dir2))}'.
 ///
-/// * It can be turned into a `Configuration`, to be used at runtime.
-/// * It retains all information necessary to produce an equivalent serialized representation.
-/// * It supports updates between versions which may require more detailed information than is
-///   available in a `Configuration` (such as whether a native query was defined inline or in a
-///   file)
+/// Upgrades between different configuration format versions are version-specific functions on
+/// 'ParsedConfiguration' as well.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum ParsedConfiguration {
     Version3(version3::RawConfiguration),
@@ -35,11 +46,17 @@ impl ParsedConfiguration {
     }
 }
 
-/// A configuration type, tailored to the needs of the query/mutation/explain methods (i.e., those
-/// not to do with configuration management).
+/// The 'Configuration' type collects all the information necessary to serve queries at runtime.
+///
+/// 'ParsedConfiguration' deals with a multitude of different concrete version formats, and each
+/// version is responsible for interpreting its serialized format into the current 'Configuration'.
+/// Values of this type are produced from a 'ParsedConfiguration' using
+/// 'make_runtime_configuration'.
+///
+/// Separating 'ParsedConfiguration' and 'Configuration' simplifies the main query translation
+/// logic by placing the responsibility of dealing with configuration format evolution in
+/// 'ParsedConfiguration.
 ///
-/// This separation also decouples the implementation from things like API versioning concerns
-/// somewhat.
 #[derive(Debug)]
 pub struct Configuration {
     pub metadata: metadata::Metadata,
@@ -78,6 +95,11 @@ pub async fn parse_configuration(
     }
 }
 
+/// Turn a 'ParsedConfiguration' into a into a 'Configuration', such that it may be used in main
+/// NDC business logic.
+///
+/// Each concrete supported version implementation is responsible for interpretation its format
+/// into the runtime configuration.
 pub fn make_runtime_configuration(
     parsed_config: ParsedConfiguration,
     environment: impl Environment,
@@ -99,6 +121,10 @@ pub async fn write_parsed_configuration(
     }
 }
 
+/// Produce an equivalent version of a parsed configuration in the latest supported version.
+///
+/// This is part of the configuration crate API to enable users to upgrade their configurations
+/// mechanically, using the ndc-postgres cli, when new versions are released..
 pub fn upgrade_to_latest_version(parsed_config: ParsedConfiguration) -> ParsedConfiguration {
     match parsed_config {
         ParsedConfiguration::Version3(v) => {

crates/configuration/src/error.rs

@@ -38,6 +38,7 @@ pub struct MultiError(pub Vec<Box<dyn std::error::Error + Send + Sync>>);
 impl Display for MultiError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         for err in &self.0 {
+            "\n".fmt(f)?;
             " * ".fmt(f)?;
             err.fmt(f)?;
             "\n".fmt(f)?;

crates/configuration/src/lib.rs

@@ -3,12 +3,58 @@ mod values;
 
 pub mod environment;
 pub mod error;
-// TODO: make these two private!
-pub mod version3;
-pub mod version4;
+mod version3;
+mod version4;
 
 pub use configuration::{
-    introspect, make_runtime_configuration, parse_configuration, upgrade_to_latest_version,
-    write_parsed_configuration, Configuration, ParsedConfiguration,
+    generate_latest_schema, introspect, make_runtime_configuration, parse_configuration,
+    upgrade_to_latest_version, write_parsed_configuration, Configuration, ParsedConfiguration,
+    DEFAULT_CONNECTION_URI_VARIABLE,
 };
 pub use values::{ConnectionUri, IsolationLevel, PoolSettings, Secret};
+
+#[cfg(test)]
+pub mod tests;
+
+#[cfg(test)]
+pub mod common {
+    use std::path::{Path, PathBuf};
+
+    pub mod postgres {
+
+        pub const CHINOOK_NDC_METADATA_PATH: &str = "static/postgres/v3-chinook-ndc-metadata";
+
+        pub const BROKEN_QUERIES_NDC_METADATA_PATH: &str =
+            "static/postgres/broken-queries-ndc-metadata";
+
+        pub const CONNECTION_URI: &str = "postgresql://postgres:password@localhost:64002";
+
+        pub const EMPTY_CONNECTION_URI: &str =
+            "postgresql://postgres:password@localhost:64002/empty";
+    }
+
+    pub mod citus {
+        pub const CHINOOK_NDC_METADATA_PATH: &str = "static/citus/v3-chinook-ndc-metadata";
+
+        pub const CONNECTION_URI: &str =
+            "postgresql://postgres:password@localhost:64004?sslmode=disable";
+    }
+
+    pub mod cockroach {
+        pub const CHINOOK_NDC_METADATA_PATH: &str = "static/cockroach/v3-chinook-ndc-metadata";
+
+        pub const CONNECTION_URI: &str = "postgresql://postgres:password@localhost:64003/defaultdb";
+    }
+
+    /// Find the project root via the crate root provided by `cargo test`,
+    /// and get our single static configuration file.
+    /// This depends on the convention that all our crates live in `/crates/<name>`
+    /// and will break in the unlikely case that we change this
+    pub fn get_path_from_project_root(ndc_metadata_path: impl AsRef<Path>) -> PathBuf {
+        let mut d = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        d.push("../../");
+        d.push(ndc_metadata_path);
+
+        d
+    }
+}