From 692ff7ae0b541362407b61e4d53e454782a80b80 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Tue, 18 Jun 2024 21:41:50 -0400 Subject: [PATCH 1/5] implement header forwarding, rework config --- Cargo.lock | 9 + crates/common/Cargo.toml | 2 + crates/common/src/client.rs | 30 +- crates/common/src/config.rs | 27 +- crates/common/src/config_file.rs | 146 +++++- crates/common/src/lib.rs | 1 + crates/common/src/schema.rs | 396 +++++++++++++++ crates/ndc-graphql-cli/src/graphql.rs | 6 +- crates/ndc-graphql-cli/src/main.rs | 41 +- crates/ndc-graphql/Cargo.toml | 1 + crates/ndc-graphql/src/connector.rs | 145 +++--- .../src/connector/configuration.rs | 100 +++- crates/ndc-graphql/src/connector/schema.rs | 418 ++++++++-------- crates/ndc-graphql/src/query_builder.rs | 456 ++++++++++-------- crates/ndc-graphql/src/query_builder/error.rs | 16 + .../src/query_builder/operation_variables.rs | 25 +- 16 files changed, 1278 insertions(+), 541 deletions(-) create mode 100644 crates/common/src/schema.rs diff --git a/Cargo.lock b/Cargo.lock index 675d238..56b0aa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -356,6 +356,8 @@ dependencies = [ name = "common" version = "0.2.8" dependencies = [ + "glob-match", + "graphql-parser", "graphql_client", "reqwest 0.12.4", "schemars", @@ -626,6 +628,12 @@ version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" +[[package]] +name = "glob-match" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9985c9503b412198aa4197559e9a318524ebc4519c229bfa05a535828c950b9d" + [[package]] name = "graphql-introspection-query" version = "0.2.0" @@ -1120,6 +1128,7 @@ version = "0.2.8" dependencies = [ "async-trait", "common", + "glob-match", "graphql-parser", "indexmap 2.2.6", "ndc-sdk", diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index 8420603..114ec0f 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -4,7 +4,9 @@ version.workspace = true edition.workspace = true [dependencies] +glob-match = "0.2.1" graphql_client = "0.14.0" +graphql-parser = "0.4.0" reqwest = { version = "0.12.3", features = [ "json", "rustls-tls", diff --git a/crates/common/src/client.rs b/crates/common/src/client.rs index 2043e40..8ba97cc 100644 --- a/crates/common/src/client.rs +++ b/crates/common/src/client.rs @@ -1,4 +1,5 @@ use crate::config::ConnectionConfig; +use glob_match::glob_match; use serde::Serialize; use std::{collections::BTreeMap, error::Error, fmt::Debug}; @@ -13,13 +14,15 @@ pub fn get_http_client( pub async fn execute_graphql( query: &str, variables: BTreeMap, + endpoint: &str, + headers: &BTreeMap, client: &reqwest::Client, - connection_config: &ConnectionConfig, -) -> Result, Box> { - let mut request = client.post(&connection_config.endpoint); + return_headers: &Vec, +) -> Result<(BTreeMap, graphql_client::Response), Box> { + let mut request = client.post(endpoint); - for (header_name, header_value) in &connection_config.headers { - request = request.header(header_name, &header_value.value); + for (header_name, header_value) in headers { + request = request.header(header_name, header_value); } let request_body = GraphQLRequest::new(query, &variables); @@ -27,6 +30,21 @@ pub async fn execute_graphql( let request = request.json(&request_body); let response = request.send().await?; + let headers = response + .headers() + .iter() + .filter_map(|(name, value)| { + for pattern in return_headers { + if glob_match(pattern, name.as_str()) { + return Some(( + name.to_string(), + value.to_str().unwrap_or_default().to_string(), + )); + } + } + None + }) + .collect(); if response.error_for_status_ref().is_err() { return Err(response.text().await?.into()); @@ -34,7 +52,7 @@ pub async fn execute_graphql( let response: graphql_client::Response = response.json().await?; - Ok(response) + Ok((headers, response)) } #[derive(Debug, Serialize)] diff --git a/crates/common/src/config.rs b/crates/common/src/config.rs index 9069ae6..878e539 100644 --- a/crates/common/src/config.rs +++ b/crates/common/src/config.rs @@ -1,14 +1,35 @@ -use crate::config_file::Header; use std::collections::BTreeMap; +use crate::{ + config_file::{RequestConfig, ResponseConfig}, + schema::SchemaDefinition, +}; + #[derive(Debug, Clone)] pub struct ServerConfig { pub connection: ConnectionConfig, - pub schema_string: String, + pub request: RequestConfig, + pub response: ResponseConfig, + pub schema: SchemaDefinition, } #[derive(Debug, Clone)] pub struct ConnectionConfig { pub endpoint: String, - pub headers: BTreeMap>, + pub headers: BTreeMap, +} + +impl ResponseConfig { + pub fn query_response_type_name(&self, query: &str) -> String { + format!( + "{}{}Query{}", + self.type_name_prefix, query, self.type_name_suffix + ) + } + pub fn mutation_response_type_name(&self, mutation: &str) -> String { + format!( + "{}{}Mutation{}", + self.type_name_prefix, mutation, self.type_name_suffix + ) + } } diff --git a/crates/common/src/config_file.rs b/crates/common/src/config_file.rs index 11129ae..0256665 100644 --- a/crates/common/src/config_file.rs +++ b/crates/common/src/config_file.rs @@ -10,7 +10,15 @@ pub const CONFIG_SCHEMA_FILE_NAME: &str = "configuration.schema.json"; pub struct ServerConfigFile { #[serde(rename = "$schema")] pub json_schema: String, + /// Connection configuration for query execution + /// Also used for introspection unless introspection connnection configuration is provided pub connection: ConnectionConfigFile, + /// Optional Connection Configuration for introspection + pub introspection: Option, + /// Optional configuration for requests + pub request: Option>>, + /// Optional configuration for responses + pub response: Option>>, } impl Default for ServerConfigFile { @@ -19,15 +27,42 @@ impl Default for ServerConfigFile { json_schema: CONFIG_SCHEMA_FILE_NAME.to_owned(), connection: ConnectionConfigFile { endpoint: ConfigValue::Value("".to_string()), - headers: BTreeMap::from_iter(vec![( - "Authorization".to_owned(), - Header { - value: ConfigValue::ValueFromEnv( - "GRAPHQL_ENDPOINT_AUTHORIZATION".to_string(), - ), - }, - )]), + headers: BTreeMap::from_iter(vec![ + ( + "Content-Type".to_owned(), + ConfigValue::Value("application/json".to_string()), + ), + ( + "Authorization".to_owned(), + ConfigValue::ValueFromEnv("GRAPHQL_ENDPOINT_AUTHORIZATION".to_string()), + ), + ]), }, + introspection: Some(ConnectionConfigFile { + endpoint: ConfigValue::Value("".to_string()), + headers: BTreeMap::from_iter(vec![ + ( + "Content-Type".to_owned(), + ConfigValue::Value("application/json".to_string()), + ), + ( + "Authorization".to_owned(), + ConfigValue::ValueFromEnv("GRAPHQL_ENDPOINT_AUTHORIZATION".to_string()), + ), + ]), + }), + request: Some(RequestConfig { + forward_headers: RequestConfig::default().forward_headers, + headers_argument: None, + headers_type_name: None, + }), + response: Some(ResponseConfig { + forward_headers: ResponseConfig::default().forward_headers, + headers_field: None, + response_field: None, + type_name_prefix: None, + type_name_suffix: None, + }), } } } @@ -36,12 +71,46 @@ impl Default for ServerConfigFile { pub struct ConnectionConfigFile { pub endpoint: ConfigValue, #[serde(skip_serializing_if = "BTreeMap::is_empty", default)] - pub headers: BTreeMap>, + pub headers: BTreeMap, } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -pub struct Header { - pub value: T, +#[serde(rename_all = "camelCase")] +pub struct RequestConfig { + /// Name of the headers argument + /// Must not conflict with any arguments of root fields in the target schema + /// Defaults to "_headers", set to a different value if there is a conflict + pub headers_argument: T, + /// Name of the headers argument type + /// Must not conflict with other types in the target schema + /// Defaults to "_HeaderMap", set to a different value if there is a conflict + pub headers_type_name: T, + /// List of headers to from the request + /// Defaults to ["*"], AKA all headers + /// Supports glob patterns eg. "X-Hasura-*" + pub forward_headers: Vec, +} +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "camelCase")] +pub struct ResponseConfig { + /// Name of the headers field in the response type + /// Defaults to "headers" + pub headers_field: T, + /// Name of the response field in the response type + /// Defaults to "response" + pub response_field: T, + /// Prefix for response type names + /// Defaults to "_" + /// Generated response type names must be unique once prefix and suffix are applied + pub type_name_prefix: T, + /// Suffix for response type names + /// Defaults to "Response" + /// Generated response type names must be unique once prefix and suffix are applied + pub type_name_suffix: T, + /// List of headers to from the response + /// Defaults to ["*"], AKA all headers + /// Supports glob patterns eg. "X-Hasura-*" + pub forward_headers: Vec, } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] @@ -51,3 +120,58 @@ pub enum ConfigValue { #[serde(rename = "valueFromEnv")] ValueFromEnv(String), } + +impl Default for RequestConfig { + fn default() -> Self { + Self { + headers_argument: "_headers".to_owned(), + headers_type_name: "_HeaderMap".to_owned(), + forward_headers: vec!["*".to_owned()], + } + } +} + +impl Default for ResponseConfig { + fn default() -> Self { + Self { + headers_field: "headers".to_owned(), + response_field: "response".to_owned(), + type_name_prefix: "_".to_owned(), + type_name_suffix: "Response".to_owned(), + forward_headers: vec!["*".to_owned()], + } + } +} + +impl From>> for RequestConfig { + fn from(value: RequestConfig>) -> Self { + RequestConfig { + headers_argument: value + .headers_argument + .unwrap_or_else(|| Self::default().headers_argument), + headers_type_name: value + .headers_type_name + .unwrap_or_else(|| Self::default().headers_type_name), + forward_headers: value.forward_headers, + } + } +} +impl From>> for ResponseConfig { + fn from(value: ResponseConfig>) -> Self { + ResponseConfig { + headers_field: value + .headers_field + .unwrap_or_else(|| Self::default().headers_field), + response_field: value + .response_field + .unwrap_or_else(|| Self::default().response_field), + type_name_prefix: value + .type_name_prefix + .unwrap_or_else(|| Self::default().type_name_prefix), + type_name_suffix: value + .type_name_suffix + .unwrap_or_else(|| Self::default().type_name_suffix), + forward_headers: value.forward_headers, + } + } +} diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 8157250..07b4bcd 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -1,3 +1,4 @@ pub mod client; pub mod config; pub mod config_file; +pub mod schema; diff --git a/crates/common/src/schema.rs b/crates/common/src/schema.rs new file mode 100644 index 0000000..9bb8ba7 --- /dev/null +++ b/crates/common/src/schema.rs @@ -0,0 +1,396 @@ +use crate::config_file::{RequestConfig, ResponseConfig}; +use graphql_parser::schema; +use std::{collections::BTreeMap, fmt::Display}; + +#[derive(Debug, Clone)] +pub struct SchemaDefinition { + pub query_fields: BTreeMap, + pub mutation_fields: BTreeMap, + pub definitions: BTreeMap, +} + +impl SchemaDefinition { + pub fn new( + schema_document: &schema::Document<'_, String>, + request_config: &RequestConfig, + response_config: &ResponseConfig, + ) -> Result { + let schema_definition = schema_document + .definitions + .iter() + .find_map(|def| match def { + schema::Definition::SchemaDefinition(schema) => Some(schema), + schema::Definition::TypeDefinition(_) + | schema::Definition::TypeExtension(_) + | schema::Definition::DirectiveDefinition(_) => None, + }) + .ok_or_else(|| SchemaDefinitionError::MissingSchemaType)?; + + // note: if there are duplicate definitions, the last one will stick. + let definitions: BTreeMap<_, _> = schema_document + .definitions + .iter() + .filter_map(|definition| match definition { + schema::Definition::SchemaDefinition(_) => None, + schema::Definition::DirectiveDefinition(_) => None, + schema::Definition::TypeExtension(_) => None, + schema::Definition::TypeDefinition(type_definition) => match type_definition { + schema::TypeDefinition::Union(_) => None, + schema::TypeDefinition::Interface(_) => None, + schema::TypeDefinition::Scalar(scalar) => Some(TypeDef::new_scalar(scalar)), + schema::TypeDefinition::Object(object) => { + // skip query, mutation, subscription types + if schema_definition + .query + .as_ref() + .is_some_and(|query_type| query_type == &object.name) + || schema_definition + .subscription + .as_ref() + .is_some_and(|subscription_type| subscription_type == &object.name) + || schema_definition + .mutation + .as_ref() + .is_some_and(|mutation_type| mutation_type == &object.name) + { + None + } else { + Some(TypeDef::new_object(object)) + } + } + schema::TypeDefinition::Enum(enum_definition) => { + Some(TypeDef::new_enum(enum_definition)) + } + schema::TypeDefinition::InputObject(input_object) => { + Some(TypeDef::new_input_object(input_object)) + } + }, + }) + .collect(); + + if definitions.contains_key(&request_config.headers_type_name) { + return Err(SchemaDefinitionError::HeaderTypeNameConflict( + request_config.headers_type_name.to_owned(), + )); + } + + let query_type = schema_document + .definitions + .iter() + .find_map(|def| match def { + schema::Definition::TypeDefinition(schema::TypeDefinition::Object(query_type)) + if schema_definition + .query + .as_ref() + .is_some_and(|query_type_name| query_type_name == &query_type.name) => + { + Some(query_type) + } + _ => None, + }); + let mut query_fields = BTreeMap::new(); + + if let Some(query_type) = query_type { + for field in &query_type.fields { + let query_field = field.name.to_owned(); + let response_type = response_config.query_response_type_name(&query_field); + + if definitions.contains_key(&response_type) { + return Err(SchemaDefinitionError::QueryResponseTypeConflict { + query_field, + response_type, + }); + } + + let field_definition = ObjectFieldDefinition::new(field); + + if field_definition + .arguments + .contains_key(&request_config.headers_argument) + { + return Err(SchemaDefinitionError::QueryHeaderArgumentConflict { + query_field, + headers_argument: request_config.headers_argument.to_owned(), + }); + } + + query_fields.insert(field.name.to_owned(), field_definition); + } + } + + let mutation_type = + schema_document + .definitions + .iter() + .find_map(|def| match def { + schema::Definition::TypeDefinition(schema::TypeDefinition::Object( + mutation_type, + )) if schema_definition.mutation.as_ref().is_some_and( + |mutation_type_name| mutation_type_name == &mutation_type.name, + ) => + { + Some(mutation_type) + } + _ => None, + }); + let mut mutation_fields = BTreeMap::new(); + + if let Some(mutation_type) = mutation_type { + for field in &mutation_type.fields { + let mutation_field = field.name.to_owned(); + let response_type = response_config.mutation_response_type_name(&mutation_field); + + if definitions.contains_key(&response_type) { + return Err(SchemaDefinitionError::MutationResponseTypeConflict { + mutation_field, + response_type, + }); + } + + let field_definition = ObjectFieldDefinition::new(field); + + if field_definition + .arguments + .contains_key(&request_config.headers_argument) + { + return Err(SchemaDefinitionError::MutationHeaderArgumentConflict { + mutation_field, + headers_argument: request_config.headers_argument.to_owned(), + }); + } + + mutation_fields.insert(field.name.to_owned(), field_definition); + } + } + + Ok(Self { + query_fields, + mutation_fields, + definitions, + }) + } +} + +#[derive(Debug, Clone)] +pub enum TypeRef { + Named(String), + List(Box), + NonNull(Box), +} + +impl TypeRef { + fn new(type_reference: &schema::Type) -> Self { + match type_reference { + schema::Type::NamedType(name) => Self::Named(name.to_owned()), + schema::Type::ListType(underlying) => Self::List(Box::new(Self::new(&underlying))), + schema::Type::NonNullType(underlying) => { + Self::NonNull(Box::new(Self::new(&underlying))) + } + } + } + pub fn name(&self) -> &str { + match self { + TypeRef::Named(n) => n.as_str(), + TypeRef::List(underlying) | TypeRef::NonNull(underlying) => underlying.name(), + } + } +} + +#[derive(Debug, Clone)] +pub enum TypeDef { + Scalar { + description: Option, + }, + Enum { + values: Vec, + description: Option, + }, + Object { + fields: BTreeMap, + description: Option, + }, + InputObject { + fields: BTreeMap, + description: Option, + }, +} + +impl TypeDef { + fn new_scalar(scalar_definition: &schema::ScalarType) -> (String, Self) { + ( + scalar_definition.name.to_owned(), + Self::Scalar { + description: scalar_definition.description.to_owned(), + }, + ) + } + fn new_enum(enum_definition: &schema::EnumType) -> (String, Self) { + ( + enum_definition.name.to_owned(), + Self::Enum { + values: enum_definition + .values + .iter() + .map(|value| EnumValueDefinition::new(value)) + .collect(), + description: enum_definition.description.to_owned(), + }, + ) + } + fn new_object(object_definition: &schema::ObjectType) -> (String, Self) { + ( + object_definition.name.to_owned(), + Self::Object { + fields: object_definition + .fields + .iter() + .map(|field| (field.name.to_owned(), ObjectFieldDefinition::new(field))) + .collect(), + description: object_definition.description.to_owned(), + }, + ) + } + fn new_input_object( + input_object_definition: &schema::InputObjectType, + ) -> (String, Self) { + ( + input_object_definition.name.to_owned(), + Self::InputObject { + fields: input_object_definition + .fields + .iter() + .map(|field| { + ( + field.name.to_owned(), + InputObjectFieldDefinition::new(field), + ) + }) + .collect(), + description: input_object_definition.description.to_owned(), + }, + ) + } +} + +#[derive(Debug, Clone)] +pub struct EnumValueDefinition { + pub name: String, + pub description: Option, +} + +impl EnumValueDefinition { + fn new(value: &schema::EnumValue) -> Self { + Self { + name: value.name.to_owned(), + description: value.description.to_owned(), + } + } +} + +#[derive(Debug, Clone)] +pub struct ObjectFieldDefinition { + pub r#type: TypeRef, + pub arguments: BTreeMap, + pub description: Option, +} + +impl ObjectFieldDefinition { + fn new(field: &schema::Field) -> Self { + Self { + r#type: TypeRef::new(&field.field_type), + arguments: field + .arguments + .iter() + .map(|argument| { + ( + argument.name.to_owned(), + ObjectFieldArgumentDefinition::new(argument), + ) + }) + .collect(), + description: field.description.to_owned(), + } + } +} + +#[derive(Debug, Clone)] +pub struct ObjectFieldArgumentDefinition { + pub r#type: TypeRef, + pub description: Option, +} + +impl ObjectFieldArgumentDefinition { + fn new(argument: &schema::InputValue) -> Self { + Self { + r#type: TypeRef::new(&argument.value_type), + description: argument.description.to_owned(), + } + } +} + +#[derive(Debug, Clone)] +pub struct InputObjectFieldDefinition { + pub r#type: TypeRef, + pub description: Option, +} + +impl InputObjectFieldDefinition { + fn new(field: &schema::InputValue) -> Self { + Self { + r#type: TypeRef::new(&field.value_type), + description: field.description.to_owned(), + } + } +} + +#[derive(Debug, Clone)] +pub enum SchemaDefinitionError { + MissingSchemaType, + HeaderTypeNameConflict(String), + QueryHeaderArgumentConflict { + query_field: String, + headers_argument: String, + }, + MutationHeaderArgumentConflict { + mutation_field: String, + headers_argument: String, + }, + QueryResponseTypeConflict { + query_field: String, + response_type: String, + }, + MutationResponseTypeConflict { + mutation_field: String, + response_type: String, + }, +} + +impl std::error::Error for SchemaDefinitionError {} + +impl Display for SchemaDefinitionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + SchemaDefinitionError::MissingSchemaType => write!( + f, + "Missing Schema Type: expected schema file with schema definition" + ), + SchemaDefinitionError::HeaderTypeNameConflict(name) => write!(f, "HeaderMap name conflict: Another type with name {name} exists. Change the name under request.headerTypeName"), + SchemaDefinitionError::QueryHeaderArgumentConflict { + query_field, + headers_argument, + } => write!(f, "Query Headers argument conflict: Query field {query_field} has an argument with name {headers_argument}. Change the headers argument name under request.headerArgument"), + SchemaDefinitionError::MutationHeaderArgumentConflict { + mutation_field, + headers_argument, + } => write!(f, "Mutation Headers argument conflict: Mutation field {mutation_field} has an argument with name {headers_argument}. Change the headers argument name under request.headerArgument"), + SchemaDefinitionError::QueryResponseTypeConflict { + query_field, + response_type, + } => write!(f, "ResponseType name conflict for Query field {query_field}: A type with name {response_type} already exist. Change the response typename prefix or suffix under response.typeNamePrefix or response.typeNameSuffix"), + SchemaDefinitionError::MutationResponseTypeConflict { + mutation_field, + response_type, + } => write!(f, "ResponseType name conflict for Mutation field {mutation_field}: A type with name {response_type} already exist. Change the response typename prefix or suffix under response.typeNamePrefix or response.typeNameSuffix"), + } + } +} diff --git a/crates/ndc-graphql-cli/src/graphql.rs b/crates/ndc-graphql-cli/src/graphql.rs index 58d2966..e992d17 100644 --- a/crates/ndc-graphql-cli/src/graphql.rs +++ b/crates/ndc-graphql-cli/src/graphql.rs @@ -24,11 +24,13 @@ pub async fn execute_graphql_introspection( let introspection_query = include_str!("./graphql/introspection_query.graphql"); - let introspection = execute_graphql::( + let (_, introspection) = execute_graphql::( &introspection_query, BTreeMap::new(), + &connection.endpoint, + &connection.headers, &client, - &connection, + &vec![], ) .await?; diff --git a/crates/ndc-graphql-cli/src/main.rs b/crates/ndc-graphql-cli/src/main.rs index 2d52a43..df8ac78 100644 --- a/crates/ndc-graphql-cli/src/main.rs +++ b/crates/ndc-graphql-cli/src/main.rs @@ -4,9 +4,9 @@ use clap::{Parser, Subcommand, ValueEnum}; use common::{ config::ConnectionConfig, config_file::{ - ConfigValue, Header, ServerConfigFile, CONFIG_FILE_NAME, CONFIG_SCHEMA_FILE_NAME, - SCHEMA_FILE_NAME, + ConfigValue, ServerConfigFile, CONFIG_FILE_NAME, CONFIG_SCHEMA_FILE_NAME, SCHEMA_FILE_NAME, }, + schema::SchemaDefinition, }; use graphql::{execute_graphql_introspection, schema_from_introspection}; use graphql_parser::schema; @@ -94,7 +94,24 @@ async fn main() -> Result<(), Box> { update_config(&context_path).await?; } Command::Validate {} => { - let _schema_document = read_schema_file(&context_path).await?; + let config_file = read_config_file(&context_path) + .await? + .ok_or_else(|| format!("Could not find {CONFIG_FILE_NAME}"))?; + let schema_document = read_schema_file(&context_path) + .await? + .ok_or_else(|| format!("Could not find {SCHEMA_FILE_NAME}"))?; + + let request_config = config_file + .request + .map(|request| request.into()) + .unwrap_or_default(); + let response_config = config_file + .response + .map(|response| response.into()) + .unwrap_or_default(); + + let _schema = + SchemaDefinition::new(&schema_document, &request_config, &response_config)?; } Command::Watch {} => { todo!("implement watch command") @@ -172,19 +189,18 @@ async fn update_config(context_path: &PathBuf) -> Result<(), Box> { } }?; + // CLI uses the introspection connection if available + let connection_file = config_file + .introspection + .unwrap_or_else(|| config_file.connection); + let connection = ConnectionConfig { - endpoint: read_config_value(&config_file.connection.endpoint)?, - headers: config_file - .connection + endpoint: read_config_value(&connection_file.endpoint)?, + headers: connection_file .headers .iter() .map(|(header_name, header_value)| { - Ok(( - header_name.to_owned(), - Header { - value: read_config_value(&header_value.value)?, - }, - )) + Ok((header_name.to_owned(), read_config_value(&header_value)?)) }) .collect::>()?, }; @@ -210,6 +226,7 @@ fn read_config_value(value: &ConfigValue) -> Result } #[tokio::test] +#[ignore] async fn update_configuration_directory() { update_config(&std::path::Path::new("../../config").to_path_buf()) .await diff --git a/crates/ndc-graphql/Cargo.toml b/crates/ndc-graphql/Cargo.toml index 36feca5..cd0404d 100644 --- a/crates/ndc-graphql/Cargo.toml +++ b/crates/ndc-graphql/Cargo.toml @@ -6,6 +6,7 @@ edition.workspace = true [dependencies] async-trait = "0.1.78" common = { path = "../common" } +glob-match = "0.2.1" graphql-parser = "0.4.0" indexmap = "2.1.0" ndc-sdk = { git = "https://github.com/hasura/ndc-sdk-rs", tag = "v0.1.3", package = "ndc-sdk", features = [ diff --git a/crates/ndc-graphql/src/connector.rs b/crates/ndc-graphql/src/connector.rs index eb2a81f..71ea8ea 100644 --- a/crates/ndc-graphql/src/connector.rs +++ b/crates/ndc-graphql/src/connector.rs @@ -1,11 +1,10 @@ -use std::{collections::BTreeMap, mem, path::Path}; - +use self::{configuration::read_configuration, state::ServerState}; +use crate::query_builder::{build_mutation_document, build_query_document}; use async_trait::async_trait; use common::{ client::{execute_graphql, GraphQLRequest}, config::ServerConfig, }; -use graphql_parser::{parse_schema, schema::Document}; use indexmap::IndexMap; use ndc_sdk::{ connector::{ @@ -17,13 +16,10 @@ use ndc_sdk::{ self, CapabilitiesResponse, LeafCapability, MutationOperationResults, RowFieldValue, RowSet, }, }; -use schema::schema_from_graphql_document; +use schema::schema_response; +use std::{collections::BTreeMap, mem, path::Path}; use tracing::Instrument; -use crate::query_builder::{build_mutation_document, build_query_document}; - -use self::{configuration::read_configuration, state::ServerState}; - mod configuration; mod schema; mod state; @@ -95,11 +91,7 @@ impl Connector for GraphQLConnector { async fn get_schema( configuration: &Self::Configuration, ) -> Result, SchemaError> { - let schema_document = parse_schema(&configuration.schema_string) - .map_err(|err| SchemaError::Other(err.to_string().into()))?; - Ok(JsonResponse::Value(schema_from_graphql_document( - &schema_document, - ))) + Ok(JsonResponse::Value(schema_response(configuration))) } async fn query_explain( @@ -107,23 +99,22 @@ impl Connector for GraphQLConnector { _state: &Self::State, request: models::QueryRequest, ) -> Result, ExplainError> { - let schema_document = tracing::info_span!( - "Parse GraphQL Schema Document", - internal.visibility = "user" - ) - .in_scope(|| -> Result, _> { parse_schema(&configuration.schema_string) }) - .map_err(|err| ExplainError::Other(err.to_string().into()))?; + let operation = tracing::info_span!("Build Query Document", internal.visibility = "user") + .in_scope(|| build_query_document(&request, &configuration))?; - let (document, variables) = - tracing::info_span!("Build Query Document", internal.visibility = "user") - .in_scope(|| build_query_document(&request, &schema_document))?; - - let query = serde_json::to_string_pretty(&GraphQLRequest::new(&document, &variables)) - .map_err(|err| ExplainError::InvalidRequest(err.to_string()))?; + let query = serde_json::to_string_pretty(&GraphQLRequest::new( + &operation.query, + &operation.variables, + )) + .map_err(|err| ExplainError::InvalidRequest(err.to_string()))?; let details = BTreeMap::from_iter(vec![ - ("SQL Query".to_string(), document.to_owned()), + ("SQL Query".to_string(), operation.query), ("Execution Plan".to_string(), query), + ( + "Headers".to_string(), + serde_json::to_string(&operation.headers).expect("should convert headers to json"), + ), ]); Ok(JsonResponse::Value(models::ExplainResponse { details })) @@ -134,22 +125,23 @@ impl Connector for GraphQLConnector { _state: &Self::State, request: models::MutationRequest, ) -> Result, ExplainError> { - let schema_document = tracing::info_span!( - "Parse GraphQL Schema Document", - internal.visibility = "user" - ) - .in_scope(|| -> Result, _> { parse_schema(&configuration.schema_string) }) - .map_err(|err| ExplainError::Other(err.to_string().into()))?; - let (document, variables) = + let operation = tracing::info_span!("Build Mutation Document", internal.visibility = "user") - .in_scope(|| build_mutation_document(&request, &schema_document))?; + .in_scope(|| build_mutation_document(&request, &configuration))?; - let query = serde_json::to_string_pretty(&GraphQLRequest::new(&document, &variables)) - .map_err(|err| ExplainError::InvalidRequest(err.to_string()))?; + let query = serde_json::to_string_pretty(&GraphQLRequest::new( + &operation.query, + &operation.variables, + )) + .map_err(|err| ExplainError::InvalidRequest(err.to_string()))?; let details = BTreeMap::from_iter(vec![ - ("SQL Query".to_string(), document.to_owned()), + ("SQL Query".to_string(), operation.query), ("Execution Plan".to_string(), query), + ( + "Headers".to_string(), + serde_json::to_string(&operation.headers).expect("should convert headers to json"), + ), ]); Ok(JsonResponse::Value(models::ExplainResponse { details })) @@ -160,16 +152,9 @@ impl Connector for GraphQLConnector { state: &Self::State, request: models::MutationRequest, ) -> Result, MutationError> { - let schema_document = tracing::info_span!( - "Parse GraphQL Schema Document", - internal.visibility = "user" - ) - .in_scope(|| -> Result, _> { parse_schema(&configuration.schema_string) }) - .map_err(|err| MutationError::Other(err.to_string().into()))?; - - let (document, variables) = + let operation = tracing::info_span!("Build Mutation Document", internal.visibility = "user") - .in_scope(|| build_mutation_document(&request, &schema_document))?; + .in_scope(|| build_mutation_document(&request, &configuration))?; let client = state .client(&configuration) @@ -179,11 +164,13 @@ impl Connector for GraphQLConnector { let execution_span = tracing::info_span!("Execute GraphQL Mutation", internal.visibility = "user"); - let response = execute_graphql::( - &document.to_string(), - variables, + let (headers, response) = execute_graphql::( + &operation.query, + operation.variables, + &configuration.connection.endpoint, + &operation.headers, &client, - &configuration.connection, + &configuration.response.forward_headers, ) .instrument(execution_span) .await @@ -202,18 +189,26 @@ impl Connector for GraphQLConnector { .iter() .enumerate() .map(|(index, operation)| match operation { - models::MutationOperation::Procedure { .. } => { + models::MutationOperation::Procedure { .. } => Ok({ let alias = format!("procedure_{index}"); - // data object keys will only get consumed once, avoid unecessary cloning let result = data .get_mut(alias) .map(|val| mem::replace(val, serde_json::Value::Null)) .unwrap_or(serde_json::Value::Null); - - MutationOperationResults::Procedure { result } - } + let response = BTreeMap::from_iter(vec![ + ( + configuration.response.headers_field.to_string(), + serde_json::to_value(&headers)?, + ), + (configuration.response.response_field.to_string(), result), + ]); + MutationOperationResults::Procedure { + result: serde_json::to_value(response)?, + } + }), }) - .collect(); + .collect::, serde_json::Error>>() + .map_err(|err| MutationError::Other(err.into()))?; Ok(JsonResponse::Value(models::MutationResponse { operation_results, @@ -231,16 +226,8 @@ impl Connector for GraphQLConnector { state: &Self::State, request: models::QueryRequest, ) -> Result, QueryError> { - let schema_document = tracing::info_span!( - "Parse GraphQL Schema Document", - internal.visibility = "user" - ) - .in_scope(|| -> Result, _> { parse_schema(&configuration.schema_string) }) - .map_err(|err| QueryError::Other(err.to_string().into()))?; - - let (document, variables) = - tracing::info_span!("Build Query Document", internal.visibility = "user") - .in_scope(|| build_query_document(&request, &schema_document))?; + let operation = tracing::info_span!("Build Query Document", internal.visibility = "user") + .in_scope(|| build_query_document(&request, &configuration))?; let client = state .client(&configuration) @@ -250,11 +237,13 @@ impl Connector for GraphQLConnector { let execution_span = tracing::info_span!("Execute GraphQL Query", internal.visibility = "user"); - let response = execute_graphql::>( - &document.to_string(), - variables, + let (headers, response) = execute_graphql::>( + &operation.query, + operation.variables, + &configuration.connection.endpoint, + &operation.headers, &client, - &configuration.connection, + &configuration.response.forward_headers, ) .instrument(execution_span) .await @@ -268,9 +257,23 @@ impl Connector for GraphQLConnector { .into(), )) } else if let Some(data) = response.data { + let headers = + serde_json::to_value(headers).map_err(|err| QueryError::Other(err.into()))?; + let data = + serde_json::to_value(data).map_err(|err| QueryError::Other(err.into()))?; + Ok(JsonResponse::Value(models::QueryResponse(vec![RowSet { aggregates: None, - rows: Some(vec![data]), + rows: Some(vec![IndexMap::from_iter(vec![ + ( + configuration.response.headers_field.to_string(), + RowFieldValue(headers), + ), + ( + configuration.response.response_field.to_string(), + RowFieldValue(data), + ), + ])]), }]))) } else { Err(QueryError::UnprocessableContent( diff --git a/crates/ndc-graphql/src/connector/configuration.rs b/crates/ndc-graphql/src/connector/configuration.rs index 0a544ea..ad74506 100644 --- a/crates/ndc-graphql/src/connector/configuration.rs +++ b/crates/ndc-graphql/src/connector/configuration.rs @@ -2,7 +2,8 @@ use std::{env, iter::once, path::PathBuf}; use common::{ config::{ConnectionConfig, ServerConfig}, - config_file::{ConfigValue, Header, ServerConfigFile, CONFIG_FILE_NAME, SCHEMA_FILE_NAME}, + config_file::{ConfigValue, ServerConfigFile, CONFIG_FILE_NAME, SCHEMA_FILE_NAME}, + schema::SchemaDefinition, }; use graphql_parser::parse_schema; use ndc_sdk::connector::{InvalidNode, InvalidNodes, KeyOrIndex, LocatedError, ParseError}; @@ -23,25 +24,40 @@ pub async fn read_configuration(context_path: &PathBuf) -> Result Result>()?, }, + request: request_config, + response: response_config, + // request: RequestConfig { + // headers_argument: config_file + // .request + // .as_ref() + // .and_then(|request| request.headers_argument.as_ref()) + // .unwrap_or(&"_headers".to_string()) + // .to_owned(), + // headers_type_name: config_file + // .request + // .as_ref() + // .and_then(|request| request.headers_type_name.as_ref()) + // .unwrap_or(&"_HeaderMap".to_string()) + // .to_owned(), + // forward_headers: config_file + // .request + // .as_ref() + // .map(|response| response.forward_headers) + // .unwrap_or(vec![]), + // }, + // response: ResponseConfig { + // headers_field: config_file + // .response + // .as_ref() + // .and_then(|response| response.headers_field.as_ref()) + // .unwrap_or(&"headers".to_string()) + // .to_owned(), + // response_field: config_file + // .response + // .as_ref() + // .and_then(|response| response.response_field.as_ref()) + // .unwrap_or(&"response".to_string()) + // .to_owned(), + // typename_prefix: config_file + // .response + // .as_ref() + // .and_then(|response| response.typename_prefix.as_ref()) + // .unwrap_or(&"_".to_string()) + // .to_owned(), + // typename_suffix: config_file + // .response + // .as_ref() + // .and_then(|response| response.typename_suffix.as_ref()) + // .unwrap_or(&"Response".to_string()) + // .to_owned(), + // forward_headers: config_file + // .response + // .as_ref() + // .map(|response| response.forward_headers) + // .unwrap_or(vec![]), + // }, }; Ok(config) diff --git a/crates/ndc-graphql/src/connector/schema.rs b/crates/ndc-graphql/src/connector/schema.rs index 7359944..7c1b4be 100644 --- a/crates/ndc-graphql/src/connector/schema.rs +++ b/crates/ndc-graphql/src/connector/schema.rs @@ -1,227 +1,176 @@ -use graphql_parser::schema; +use common::{ + config::ServerConfig, + schema::{ + InputObjectFieldDefinition, ObjectFieldArgumentDefinition, ObjectFieldDefinition, TypeRef, + }, +}; use ndc_sdk::models; -use std::collections::BTreeMap; +use std::{collections::BTreeMap, iter}; -pub fn schema_from_graphql_document( - schema_document: &schema::Document<'_, String>, -) -> models::SchemaResponse { - let schema_definition = schema_document +pub fn schema_response(configuration: &ServerConfig) -> models::SchemaResponse { + let mut scalar_types: BTreeMap<_, _> = configuration + .schema .definitions .iter() - .find_map(|def| match def { - schema::Definition::SchemaDefinition(schema) => Some(schema), - schema::Definition::TypeDefinition(_) - | schema::Definition::TypeExtension(_) - | schema::Definition::DirectiveDefinition(_) => None, - }) - .expect("schema type"); - - let scalar_types = schema_document - .definitions - .iter() - .filter_map(|def| { - // todo: enums - if let schema::Definition::TypeDefinition(schema::TypeDefinition::Scalar(s)) = def { - let type_name = s.name.to_owned(); - - let type_definition = models::ScalarType { - representation: None, // todo: figure out type representation for scalar types + .filter_map(|(name, typedef)| match typedef { + common::schema::TypeDef::Object { .. } + | common::schema::TypeDef::InputObject { .. } => None, + common::schema::TypeDef::Scalar { description: _ } => Some(( + name.to_owned(), + models::ScalarType { + representation: None, aggregate_functions: BTreeMap::new(), comparison_operators: BTreeMap::new(), - }; - - Some((type_name, type_definition)) - } else if let schema::Definition::TypeDefinition(schema::TypeDefinition::Enum(e)) = def - { - let type_name = e.name.to_owned(); - - let type_definition = models::ScalarType { - representation: None, // todo: figure out type representation for enum types + }, + )), + common::schema::TypeDef::Enum { + values, + description: _, + } => Some(( + name.to_owned(), + models::ScalarType { + representation: Some(models::TypeRepresentation::Enum { + one_of: values.iter().map(|value| value.name.to_owned()).collect(), + }), aggregate_functions: BTreeMap::new(), comparison_operators: BTreeMap::new(), - }; - - Some((type_name, type_definition)) - } else { - None - } + }, + )), }) .collect(); - let object_types = schema_document + scalar_types.insert( + configuration.request.headers_type_name.to_owned(), + models::ScalarType { + representation: Some(models::TypeRepresentation::JSON), + aggregate_functions: BTreeMap::new(), + comparison_operators: BTreeMap::new(), + }, + ); + + let mut object_types: BTreeMap<_, _> = configuration + .schema .definitions .iter() - .filter_map(|def| { - if let schema::Definition::TypeDefinition(schema::TypeDefinition::Object(object_type)) = - def - { - let name = object_type.name.to_owned(); - - // skip query, mutation, subscription types - if schema_definition - .query - .as_ref() - .is_some_and(|query_type| query_type == &name) - || schema_definition - .subscription - .as_ref() - .is_some_and(|subscription_type| subscription_type == &name) - || schema_definition - .mutation - .as_ref() - .is_some_and(|mutation_type| mutation_type == &name) - { - return None; - } - - let fields = object_type - .fields - .iter() - .map(|field| { - ( - field.name.to_owned(), - models::ObjectField { - description: field.description.to_owned(), - r#type: to_ndc_type(&field.field_type), - arguments: field - .arguments - .iter() - .map(|arg| { - ( - arg.name.to_owned(), - models::ArgumentInfo { - description: arg.description.to_owned(), - argument_type: to_ndc_type(&arg.value_type), - }, - ) - }) - .collect(), - }, - ) - }) - .collect(); - - Some(( - name, - models::ObjectType { - description: object_type.description.to_owned(), - fields, - }, - )) - } else if let schema::Definition::TypeDefinition(schema::TypeDefinition::InputObject( - input_type, - )) = def - { - let name = input_type.name.to_owned(); - let fields = input_type - .fields - .iter() - .map(|field| { - ( - field.name.to_owned(), - models::ObjectField { - description: field.description.to_owned(), - r#type: to_ndc_type(&field.value_type), - arguments: BTreeMap::new(), - }, - ) - }) - .collect(); - - Some(( - name, - models::ObjectType { - description: input_type.description.to_owned(), - fields, - }, - )) - } else { - None - } + .filter_map(|(name, typedef)| match typedef { + common::schema::TypeDef::Scalar { .. } | common::schema::TypeDef::Enum { .. } => None, + common::schema::TypeDef::Object { + fields, + description, + } => Some(( + name.to_owned(), + models::ObjectType { + description: description.to_owned(), + fields: fields.iter().map(map_object_field).collect(), + }, + )), + common::schema::TypeDef::InputObject { + fields, + description, + } => Some(( + name.to_owned(), + models::ObjectType { + description: description.to_owned(), + fields: fields.iter().map(map_input_object_field).collect(), + }, + )), }) .collect(); - let functions = schema_document - .definitions - .iter() - .find_map(|def| match def { - schema::Definition::TypeDefinition(schema::TypeDefinition::Object(query_type)) - if schema_definition - .query - .as_ref() - .is_some_and(|query_type_name| query_type_name == &query_type.name) => - { - Some(query_type) + let response_type = + |field: &ObjectFieldDefinition, operation_type: &str, operation_name: &str| { + models::ObjectType { + description: Some(format!( + "Response type for {operation_type} {operation_name}" + )), + fields: BTreeMap::from_iter(vec![ + ( + configuration.response.headers_field.to_owned(), + models::ObjectField { + description: None, + r#type: models::Type::Named { + name: configuration.request.headers_type_name.to_owned(), + }, + arguments: BTreeMap::new(), + }, + ), + ( + configuration.response.response_field.to_owned(), + models::ObjectField { + description: None, + r#type: typeref_to_ndc_type(&field.r#type), + arguments: BTreeMap::new(), + }, + ), + ]), } - _ => None, - }) - .map(|query_type| { - query_type - .fields + }; + + let mut functions = vec![]; + + for (name, field) in &configuration.schema.query_fields { + let response_type_name = configuration.response.query_response_type_name(&name); + + object_types.insert( + response_type_name.clone(), + response_type(field, "function", name), + ); + + functions.push(models::FunctionInfo { + name: name.to_owned(), + description: field.description.to_owned(), + arguments: field + .arguments .iter() - .map(|field| models::FunctionInfo { - name: field.name.to_owned(), - description: field.description.to_owned(), - arguments: field - .arguments - .iter() - .map(|arg| { - ( - arg.name.to_owned(), - models::ArgumentInfo { - description: arg.description.to_owned(), - argument_type: to_ndc_type(&arg.value_type), - }, - ) - }) - .collect(), - result_type: to_ndc_type(&field.field_type), - }) - .collect() - }) - .unwrap_or_default(); + .map(map_argument) + .chain(iter::once(( + configuration.request.headers_argument.to_owned(), + models::ArgumentInfo { + description: None, + argument_type: models::Type::Named { + name: configuration.request.headers_type_name.to_owned(), + }, + }, + ))) + .collect(), + result_type: models::Type::Named { + name: response_type_name, + }, + }); + } - let procedures = schema_document - .definitions - .iter() - .find_map(|def| match def { - schema::Definition::TypeDefinition(schema::TypeDefinition::Object(mutation_type)) - if schema_definition - .mutation - .as_ref() - .is_some_and(|mutation_type_name| { - mutation_type_name == &mutation_type.name - }) => - { - Some(mutation_type) - } - _ => None, - }) - .map(|mutation_type| { - mutation_type - .fields + let mut procedures = vec![]; + + for (name, field) in &configuration.schema.mutation_fields { + let response_type_name = configuration.response.mutation_response_type_name(&name); + + object_types.insert( + response_type_name.clone(), + response_type(field, "procedure", name), + ); + + procedures.push(models::ProcedureInfo { + name: name.to_owned(), + description: field.description.to_owned(), + arguments: field + .arguments .iter() - .map(|field| models::ProcedureInfo { - name: field.name.to_owned(), - description: field.description.to_owned(), - arguments: field - .arguments - .iter() - .map(|arg| { - ( - arg.name.to_owned(), - models::ArgumentInfo { - description: arg.description.to_owned(), - argument_type: to_ndc_type(&arg.value_type), - }, - ) - }) - .collect(), - result_type: to_ndc_type(&field.field_type), - }) - .collect() - }) - .unwrap_or_default(); + .map(map_argument) + .chain(iter::once(( + configuration.request.headers_argument.to_owned(), + models::ArgumentInfo { + description: None, + argument_type: models::Type::Named { + name: configuration.request.headers_type_name.to_owned(), + }, + }, + ))) + .collect(), + result_type: models::Type::Named { + name: response_type_name, + }, + }); + } models::SchemaResponse { scalar_types, @@ -232,24 +181,61 @@ pub fn schema_from_graphql_document( } } -fn to_ndc_type(field_type: &schema::Type) -> models::Type { - match field_type { - schema::Type::NamedType(name) => models::Type::Nullable { +fn map_object_field( + (name, field): (&String, &ObjectFieldDefinition), +) -> (String, models::ObjectField) { + ( + name.to_owned(), + models::ObjectField { + description: field.description.to_owned(), + r#type: typeref_to_ndc_type(&field.r#type), + arguments: field.arguments.iter().map(map_argument).collect(), + }, + ) +} + +fn map_argument( + (name, argument): (&String, &ObjectFieldArgumentDefinition), +) -> (String, models::ArgumentInfo) { + ( + name.to_owned(), + models::ArgumentInfo { + description: argument.description.to_owned(), + argument_type: typeref_to_ndc_type(&argument.r#type), + }, + ) +} + +fn map_input_object_field( + (name, field): (&String, &InputObjectFieldDefinition), +) -> (String, models::ObjectField) { + ( + name.to_owned(), + models::ObjectField { + description: field.description.to_owned(), + r#type: typeref_to_ndc_type(&field.r#type), + arguments: BTreeMap::new(), + }, + ) +} + +fn typeref_to_ndc_type(typeref: &TypeRef) -> models::Type { + match typeref { + TypeRef::Named(name) => models::Type::Nullable { underlying_type: Box::new(models::Type::Named { name: name.into() }), }, - schema::Type::ListType(inner) => models::Type::Nullable { + TypeRef::List(inner) => models::Type::Nullable { underlying_type: Box::new(models::Type::Array { - element_type: Box::new(to_ndc_type(inner)), + element_type: Box::new(typeref_to_ndc_type(inner)), }), }, - schema::Type::NonNullType(inner) => match &**inner { - schema::Type::NamedType(name) => models::Type::Named { name: name.into() }, - schema::Type::ListType(inner) => models::Type::Array { - element_type: Box::new(to_ndc_type(inner)), + TypeRef::NonNull(inner) => match &**inner { + TypeRef::Named(name) => models::Type::Named { name: name.into() }, + TypeRef::List(inner) => models::Type::Array { + element_type: Box::new(typeref_to_ndc_type(inner)), }, - schema::Type::NonNullType(_) => { - todo!("Nested non-null (T!!) is not valid graphql. Todo: handle as error") - } + // ignore (illegal) double non-null assertions. This shouln't happen anyways + TypeRef::NonNull(_) => typeref_to_ndc_type(inner), }, } } diff --git a/crates/ndc-graphql/src/query_builder.rs b/crates/ndc-graphql/src/query_builder.rs index b92e9ae..67fe7d4 100644 --- a/crates/ndc-graphql/src/query_builder.rs +++ b/crates/ndc-graphql/src/query_builder.rs @@ -1,17 +1,19 @@ -use std::collections::BTreeMap; - +use self::{error::QueryBuilderError, operation_variables::OperationVariables}; +use common::{ + config::ServerConfig, + schema::{ObjectFieldDefinition, TypeDef}, +}; +use glob_match::glob_match; use graphql_parser::{ query::{ Definition, Document, Field, Mutation, OperationDefinition, Query, Selection, SelectionSet, - Type, Value, + Value, }, - schema::{self, InputValue, ObjectType}, Pos, }; use indexmap::IndexMap; use ndc_sdk::models::{self, Argument, NestedField}; - -use self::{error::QueryBuilderError, operation_variables::OperationVariables}; +use std::collections::BTreeMap; pub mod error; mod operation_variables; @@ -20,20 +22,19 @@ fn pos() -> Pos { Pos { line: 0, column: 0 } } +pub struct Operation { + pub query: String, + pub variables: BTreeMap, + pub headers: BTreeMap, +} + pub fn build_mutation_document<'a>( request: &models::MutationRequest, - schema_document: &'a schema::Document<'a, String>, -) -> Result<(String, BTreeMap), QueryBuilderError> { + configuration: &ServerConfig, +) -> Result { let mut variables = OperationVariables::new(); - let schema_type = schema_type(schema_document)?; - - let mutation_type_name = schema_type - .mutation - .as_ref() - .ok_or_else(|| QueryBuilderError::NoMutationType)?; - - let mutation_type = object_type(&mutation_type_name, schema_document)?; + let mut request_headers = configuration.connection.headers.clone(); let selection_set = SelectionSet { span: (pos(), pos()), @@ -48,20 +49,35 @@ pub fn build_mutation_document<'a>( fields, } => { let alias = format!("procedure_{index}"); + let field_definition = + configuration.schema.query_fields.get(name).ok_or_else(|| { + QueryBuilderError::QueryFieldNotFound { + field: name.to_owned(), + } + })?; + + let (headers, procedure_arguments) = + extract_headers(arguments, map_arg, configuration)?; + + for (name, header) in headers { + // if headers are duplicated, the last to be inserted stays + // todo: restrict what headers are forwarded here based on config + request_headers.insert(name, header.to_string()); + } + selection_set_field( &alias, &name, field_arguments( - arguments, + &procedure_arguments, |v| Ok(v.to_owned()), - &name, - mutation_type, + field_definition, &mut variables, )?, - mutation_type, - fields, + &fields, + field_definition, &mut variables, - schema_document, + configuration, ) } }) @@ -82,25 +98,22 @@ pub fn build_mutation_document<'a>( ))], }; - Ok((document.to_string(), values)) + Ok(Operation { + query: document.to_string(), + variables: values, + headers: request_headers, + }) } pub fn build_query_document<'a>( request: &models::QueryRequest, - schema_document: &'a schema::Document<'a, String>, -) -> Result<(String, BTreeMap), QueryBuilderError> { - // because all queries are commands, we can expect requests to always have this exact shape - + configuration: &ServerConfig, +) -> Result { let mut variables = OperationVariables::new(); - let schema_type = schema_type(schema_document)?; - - let query_type_name = schema_type - .query - .as_ref() - .ok_or_else(|| QueryBuilderError::NoQueryType)?; - - let query_type = object_type(&query_type_name, schema_document)?; + let (headers, request_arguments) = + extract_headers(&request.arguments, map_query_arg, configuration)?; + // because all queries are commands, we can expect requests to always have this exact shape let selection_set = SelectionSet { span: (pos(), pos()), items: request @@ -132,20 +145,22 @@ pub fn build_query_document<'a>( return Err(QueryBuilderError::Unexpected("Functions arguments should be passed to the collection, not the __value field".to_string())) } + let field_definition = configuration.schema.query_fields.get(&request.collection).ok_or_else(|| QueryBuilderError::QueryFieldNotFound { field: request.collection.to_owned() })?; + selection_set_field( alias, &request.collection, field_arguments( - &request.arguments, - map_query_arg, - &request.collection, - query_type, + &request_arguments, + map_arg, + field_definition, &mut variables, + )?, - query_type, fields, + field_definition, &mut variables, - schema_document, + configuration, ) }) .collect::>()?, @@ -163,24 +178,86 @@ pub fn build_query_document<'a>( }))], }; - Ok((document.to_string(), values)) + Ok(Operation { + query: document.to_string(), + variables: values, + headers, + }) +} + +fn extract_headers( + arguments: &BTreeMap, + map_argument: M, + configuration: &ServerConfig, +) -> Result< + ( + BTreeMap, + BTreeMap, + ), + QueryBuilderError, +> +where + M: Fn(&A) -> Result, +{ + let mut request_arguments = BTreeMap::new(); + let mut headers = BTreeMap::new(); + + for (name, argument) in arguments { + let value = map_argument(&argument)?; + + if name == &configuration.request.headers_argument { + match value { + serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::String(_) + | serde_json::Value::Array(_) => { + return Err(QueryBuilderError::MisshapenHeadersArgument( + value.to_owned(), + )) + } + serde_json::Value::Object(object) => { + for (name, value) in object { + match value { + serde_json::Value::Null + | serde_json::Value::Bool(_) + | serde_json::Value::Number(_) + | serde_json::Value::Array(_) + | serde_json::Value::Object(_) => { + return Err(QueryBuilderError::MisshapenHeadersArgument( + value.to_owned(), + )) + } + serde_json::Value::String(header) => { + for pattern in &configuration.request.forward_headers { + if glob_match(&pattern, &name) { + headers.insert(name, header); + break; + } + } + } + } + } + } + } + } else { + request_arguments.insert(name.to_owned(), value); + } + } + + Ok((headers, request_arguments)) } fn selection_set_field<'a>( alias: &str, - name: &str, + field_name: &str, arguments: Vec<(String, Value<'a, String>)>, - parent_object_type: &'a schema::ObjectType<'a, String>, fields: &Option, - variables: &mut OperationVariables<'a>, - schema_document: &'a schema::Document<'a, String>, + field_definition: &ObjectFieldDefinition, + variables: &mut OperationVariables, + configuration: &ServerConfig, ) -> Result, QueryBuilderError> { let selection_set = match fields.as_ref().map(underlying_fields) { Some(fields) => { - // if some, this is an object type - let field_type = object_field_type(&name, parent_object_type)?; - let type_name = type_name(&field_type.field_type); - let field_object_type = object_type(type_name, schema_document)?; - let items = fields .iter() .map(|(alias, field)| { @@ -197,20 +274,35 @@ fn selection_set_field<'a>( } }; + // subfield selection should only exist on object types + let field_definition = + match configuration + .schema + .definitions + .get(field_definition.r#type.name()) + { + Some(TypeDef::Object { + fields, + description: _, + }) => fields.get(name).ok_or_else(|| { + QueryBuilderError::ObjectFieldNotFound { + object: field_definition.r#type.name().to_owned(), + field: name.to_owned(), + } + }), + Some(_) | None => Err(QueryBuilderError::ObjectTypeNotFound( + field_definition.r#type.name().to_owned(), + )), + }?; + selection_set_field( alias, name, - field_arguments( - arguments, - map_query_arg, - name, - field_object_type, - variables, - )?, - field_object_type, + field_arguments(arguments, map_query_arg, field_definition, variables)?, fields, + field_definition, variables, - schema_document, + configuration, ) }) .collect::>()?; @@ -227,12 +319,12 @@ fn selection_set_field<'a>( }; Ok(Selection::Field(Field { position: pos(), - alias: if alias == name { + alias: if alias == field_name { None } else { Some(alias.to_owned()) }, - name: name.to_owned(), + name: field_name.to_owned(), arguments, directives: vec![], selection_set, @@ -241,9 +333,8 @@ fn selection_set_field<'a>( fn field_arguments<'a, A, M>( arguments: &BTreeMap, map_argument: M, - field_name: &str, - object_type: &'a ObjectType<'a, String>, - variables: &mut OperationVariables<'a>, + field_definition: &ObjectFieldDefinition, + variables: &mut OperationVariables, ) -> Result)>, QueryBuilderError> where M: Fn(&A) -> Result, @@ -251,83 +342,16 @@ where arguments .iter() .map(|(name, arg)| { - let input_value = object_field_arg_input_value(&name, field_name, object_type)?; + let input_type = &field_definition.arguments.get(name).unwrap().r#type; let value = map_argument(arg)?; - let value = variables.insert(name, value, input_value); + let value = variables.insert(name, value, input_type); Ok((name.to_owned(), value)) }) .collect() } -fn schema_type<'a>( - schema_document: &'a schema::Document<'a, String>, -) -> Result<&'a schema::SchemaDefinition<'a, String>, QueryBuilderError> { - schema_document - .definitions - .iter() - .find_map(|definition| match definition { - schema::Definition::SchemaDefinition(schema) => Some(schema), - _ => None, - }) - .ok_or_else(|| QueryBuilderError::SchemaDefinitionNotFound) -} -fn object_type<'a>( - type_name: &str, - schema_document: &'a schema::Document<'a, String>, -) -> Result<&'a schema::ObjectType<'a, String>, QueryBuilderError> { - schema_document - .definitions - .iter() - .find_map(|definition| match definition { - schema::Definition::TypeDefinition(schema::TypeDefinition::Object(object)) - if object.name == type_name => - { - Some(object) - } - _ => None, - }) - .ok_or_else(|| QueryBuilderError::ObjectTypeNotFound(type_name.to_owned())) -} -fn object_field_type<'a>( - field_name: &str, - object_type: &'a ObjectType<'a, String>, -) -> Result<&'a schema::Field<'a, String>, QueryBuilderError> { - object_type - .fields - .iter() - .find(|field| field.name == field_name) - .ok_or_else(|| QueryBuilderError::ObjectFieldNotFound { - object: object_type.name.to_owned(), - field: field_name.to_owned(), - }) -} -fn object_field_arg_input_value<'a>( - argument_name: &str, - field_name: &str, - object_type: &'a ObjectType<'a, String>, -) -> Result<&'a InputValue<'a, String>, QueryBuilderError> { - let field_type = object_field_type(field_name, object_type)?; - - field_type - .arguments - .iter() - .find(|arg| arg.name == argument_name) - .ok_or_else(|| QueryBuilderError::ArgumentNotFound { - object: object_type.name.to_owned(), - field: field_name.to_owned(), - argument: argument_name.to_owned(), - }) -} - -fn type_name<'a>(decorated_type: &'a Type) -> &'a str { - match decorated_type { - Type::NamedType(n) => n, - Type::ListType(l) => type_name(l), - Type::NonNullType(n) => type_name(n), - } -} fn map_query_arg(argument: &models::Argument) -> Result { match argument { @@ -337,6 +361,9 @@ fn map_query_arg(argument: &models::Argument) -> Result Ok(value.to_owned()), } } +fn map_arg(argument: &serde_json::Value) -> Result { + Ok(argument.to_owned()) +} fn underlying_fields(nested_field: &NestedField) -> &IndexMap { match nested_field { @@ -345,79 +372,116 @@ fn underlying_fields(nested_field: &NestedField) -> &IndexMap Result<(), Box> { - let schema_string = r#" - schema { - query: query_root - } - - - scalar Int - - scalar String - - - type query_root { - "fetch data from the table: \"test\" using primary key columns" - test_by_pk(id: Int!): test - } - - "columns and relationships of \"test\"" - type test { - id: Int! - name: String! - } - - "#; - let schema_document = graphql_parser::parse_schema(&schema_string)?; - let request = serde_json::from_value(serde_json::json!({ - "collection": "test_by_pk", - "query": { - "fields": { - "__value": { - "type": "column", - "column": "__value", +#[cfg(test)] +mod test { + use std::collections::BTreeMap; + + use common::{ + config::{ConnectionConfig, ServerConfig}, + config_file::{RequestConfig, ResponseConfig}, + schema::SchemaDefinition, + }; + use graphql_parser; + + use crate::query_builder::build_query_document; + + #[test] + fn process_query_into_expected_graphql_string() -> Result<(), Box> { + let schema_string = r#" + schema { + query: query_root + } + + + scalar Int + + scalar String + + + type query_root { + "fetch data from the table: \"test\" using primary key columns" + test_by_pk(id: Int!): test + } + + "columns and relationships of \"test\"" + type test { + id: Int! + name: String! + } + + "#; + let schema_document = graphql_parser::parse_schema(&schema_string)?; + let request_config = RequestConfig::default(); + let response_config = ResponseConfig::default(); + + let schema = SchemaDefinition::new(&schema_document, &request_config, &response_config)?; + let configuration = ServerConfig { + schema, + request: request_config, + response: response_config, + connection: ConnectionConfig { + endpoint: "".to_string(), + headers: BTreeMap::new(), + }, + }; + let request = serde_json::from_value(serde_json::json!({ + "collection": "test_by_pk", + "query": { "fields": { - "type": "object", - "fields": { - "id": { - "type": "column", - "column": "id", - "fields": null - }, - "name": { - "type": "column", - "column": "name", - "fields": null + "__value": { + "type": "column", + "column": "__value", + "fields": { + "type": "object", + "fields": { + "id": { + "type": "column", + "column": "id", + "fields": null + }, + "name": { + "type": "column", + "column": "name", + "fields": null + } + } } } } - } - } - }, - "arguments": { - "id": { - "type": "literal", - "value": 1 - } - }, - "collection_relationships": {} - }))?; + }, + "arguments": { + "_headers": { + "type": "literal", + "value": { + "Authorization": "Bearer" + } + }, + "id": { + "type": "literal", + "value": 1 + } + }, + "collection_relationships": {} + }))?; - let (document, variables) = build_query_document(&request, &schema_document)?; + let operation = build_query_document(&request, &configuration)?; - let expected_query = r#"query($arg_1_id: Int!) { + let expected_query = r#" +query($arg_1_id: Int!) { __value: test_by_pk(id: $arg_1_id) { id name } }"#; - let expected_variables = - BTreeMap::from_iter(vec![("arg_1_id".to_string(), serde_json::json!(1))]); + let expected_variables = + BTreeMap::from_iter(vec![("arg_1_id".to_string(), serde_json::json!(1))]); + let expected_headers = + BTreeMap::from_iter(vec![("Authorization".to_string(), "Bearer".to_string())]); - assert_eq!(document, expected_query); - assert_eq!(variables, expected_variables); + assert_eq!(operation.query.trim(), expected_query.trim()); + assert_eq!(operation.variables, expected_variables); + assert_eq!(operation.headers, expected_headers); - Ok(()) + Ok(()) + } } diff --git a/crates/ndc-graphql/src/query_builder/error.rs b/crates/ndc-graphql/src/query_builder/error.rs index 765c6fe..1fca98e 100644 --- a/crates/ndc-graphql/src/query_builder/error.rs +++ b/crates/ndc-graphql/src/query_builder/error.rs @@ -11,6 +11,12 @@ pub enum QueryBuilderError { NoQueryType, NoMutationType, NotSupported(String), + QueryFieldNotFound { + field: String, + }, + MutationFieldNotFound { + field: String, + }, ObjectFieldNotFound { object: String, field: String, @@ -24,6 +30,7 @@ pub enum QueryBuilderError { field: String, argument: String, }, + MisshapenHeadersArgument(serde_json::Value), Unexpected(String), } @@ -86,6 +93,15 @@ impl Display for QueryBuilderError { "Argument {argument} for field {field} not found in Object Type {object}" ), QueryBuilderError::Unexpected(s) => write!(f, "Unexpected: {s}"), + QueryBuilderError::QueryFieldNotFound { field } => { + write!(f, "Field {field} not found in Query type") + } + QueryBuilderError::MutationFieldNotFound { field } => { + write!(f, "Field {field} not found in Mutation type") + } + QueryBuilderError::MisshapenHeadersArgument(headers) => { + write!(f, "Misshapen headers argument: {}", headers.to_string()) + } } } } diff --git a/crates/ndc-graphql/src/query_builder/operation_variables.rs b/crates/ndc-graphql/src/query_builder/operation_variables.rs index 8006a04..4eed393 100644 --- a/crates/ndc-graphql/src/query_builder/operation_variables.rs +++ b/crates/ndc-graphql/src/query_builder/operation_variables.rs @@ -1,17 +1,17 @@ use std::collections::BTreeMap; +use common::schema::TypeRef; use graphql_parser::{ query::{Type, Value, VariableDefinition}, - schema::InputValue, Pos, }; -pub struct OperationVariables<'c> { - variables: BTreeMap)>, +pub struct OperationVariables { + variables: BTreeMap, variable_index: u32, } -impl<'c> OperationVariables<'c> { +impl<'c> OperationVariables { pub fn new() -> Self { Self { variables: BTreeMap::new(), @@ -22,13 +22,13 @@ impl<'c> OperationVariables<'c> { &mut self, name: &str, value: serde_json::Value, - input_value: &'c InputValue<'c, String>, + r#type: &TypeRef, ) -> Value<'c, String> { let name = format!("arg_{}_{}", self.variable_index, name); self.variable_index += 1; self.variables - .insert(name.clone(), (value, &input_value.value_type)); + .insert(name.clone(), (value, r#type.to_owned())); Value::Variable(name) } @@ -38,16 +38,25 @@ impl<'c> OperationVariables<'c> { BTreeMap, Vec>, ) { + fn typeref_to_type<'c>(typeref: TypeRef) -> Type<'c, String> { + match typeref { + TypeRef::Named(name) => Type::NamedType(name), + TypeRef::List(underlying) => Type::ListType(Box::new(typeref_to_type(*underlying))), + TypeRef::NonNull(underlying) => { + Type::NonNullType(Box::new(typeref_to_type(*underlying))) + } + } + } let (values, definitions) = self .variables .into_iter() - .map(|(alias, (value, var_type))| { + .map(|(alias, (value, typeref))| { ( (alias.clone(), value), VariableDefinition { position: Pos { line: 0, column: 0 }, name: alias, - var_type: var_type.to_owned(), + var_type: typeref_to_type(typeref), default_value: None, }, ) From 16c2d9aff6723c94d28c88d9a5b41e6328006345 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Tue, 18 Jun 2024 21:42:00 -0400 Subject: [PATCH 2/5] make clippy happy --- crates/common/src/schema.rs | 8 +-- crates/ndc-graphql-cli/src/graphql.rs | 14 ++--- crates/ndc-graphql-cli/src/main.rs | 36 +++++------ crates/ndc-graphql/src/connector.rs | 19 +++--- .../src/connector/configuration.rs | 63 +++---------------- crates/ndc-graphql/src/connector/schema.rs | 4 +- crates/ndc-graphql/src/query_builder.rs | 25 ++++---- crates/ndc-graphql/src/query_builder/error.rs | 2 +- 8 files changed, 59 insertions(+), 112 deletions(-) diff --git a/crates/common/src/schema.rs b/crates/common/src/schema.rs index 9bb8ba7..b5053cf 100644 --- a/crates/common/src/schema.rs +++ b/crates/common/src/schema.rs @@ -24,7 +24,7 @@ impl SchemaDefinition { | schema::Definition::TypeExtension(_) | schema::Definition::DirectiveDefinition(_) => None, }) - .ok_or_else(|| SchemaDefinitionError::MissingSchemaType)?; + .ok_or(SchemaDefinitionError::MissingSchemaType)?; // note: if there are duplicate definitions, the last one will stick. let definitions: BTreeMap<_, _> = schema_document @@ -182,10 +182,8 @@ impl TypeRef { fn new(type_reference: &schema::Type) -> Self { match type_reference { schema::Type::NamedType(name) => Self::Named(name.to_owned()), - schema::Type::ListType(underlying) => Self::List(Box::new(Self::new(&underlying))), - schema::Type::NonNullType(underlying) => { - Self::NonNull(Box::new(Self::new(&underlying))) - } + schema::Type::ListType(underlying) => Self::List(Box::new(Self::new(underlying))), + schema::Type::NonNullType(underlying) => Self::NonNull(Box::new(Self::new(underlying))), } } pub fn name(&self) -> &str { diff --git a/crates/ndc-graphql-cli/src/graphql.rs b/crates/ndc-graphql-cli/src/graphql.rs index e992d17..6ed6493 100644 --- a/crates/ndc-graphql-cli/src/graphql.rs +++ b/crates/ndc-graphql-cli/src/graphql.rs @@ -20,12 +20,12 @@ mod introspection; pub async fn execute_graphql_introspection( connection: &ConnectionConfig, ) -> Result, Box> { - let client = get_http_client(&connection)?; + let client = get_http_client(connection)?; let introspection_query = include_str!("./graphql/introspection_query.graphql"); let (_, introspection) = execute_graphql::( - &introspection_query, + introspection_query, BTreeMap::new(), &connection.endpoint, &connection.headers, @@ -43,7 +43,7 @@ fn pos() -> Pos { } fn is_graphql_introspection_type(name: &str) -> bool { - vec![ + [ "__Schema", "__Type", "__TypeKind", @@ -107,7 +107,7 @@ pub fn schema_from_introspection(introspection: Introspection) -> Document<'stat description: arg.description, name: arg.name, value_type: input_type(arg.r#type), - default_value: arg.default_value.map(|s| Value::String(s)), + default_value: arg.default_value.map(Value::String), directives: vec![], }) .collect(), @@ -130,7 +130,7 @@ pub fn schema_from_introspection(introspection: Introspection) -> Document<'stat description: field.description, name: field.name, value_type: input_type(field.r#type), - default_value: field.default_value.map(|v| Value::String(v)), + default_value: field.default_value.map(Value::String), directives: vec![], }) .collect(), @@ -178,7 +178,7 @@ pub fn schema_from_introspection(introspection: Introspection) -> Document<'stat description: arg.description, name: arg.name, value_type: input_type(arg.r#type), - default_value: arg.default_value.map(|s| Value::String(s)), + default_value: arg.default_value.map(Value::String), directives: vec![], }) .collect(), @@ -213,7 +213,7 @@ pub fn schema_from_introspection(introspection: Introspection) -> Document<'stat description: arg.description, name: arg.name, value_type: input_type(arg.r#type), - default_value: arg.default_value.map(|s| Value::String(s)), + default_value: arg.default_value.map(Value::String), directives: vec![], }) .collect(), diff --git a/crates/ndc-graphql-cli/src/main.rs b/crates/ndc-graphql-cli/src/main.rs index df8ac78..1d7dc1c 100644 --- a/crates/ndc-graphql-cli/src/main.rs +++ b/crates/ndc-graphql-cli/src/main.rs @@ -1,4 +1,8 @@ -use std::{env, error::Error, path::PathBuf}; +use std::{ + env, + error::Error, + path::{Path, PathBuf}, +}; use clap::{Parser, Subcommand, ValueEnum}; use common::{ @@ -122,7 +126,7 @@ async fn main() -> Result<(), Box> { } async fn write_config_file( - context_path: &PathBuf, + context_path: &Path, config: &ServerConfigFile, ) -> Result<(), Box> { let config_file_path = context_path.join(CONFIG_FILE_NAME); @@ -131,7 +135,7 @@ async fn write_config_file( Ok(()) } async fn write_schema_file( - context_path: &PathBuf, + context_path: &Path, schema: &graphql_parser::schema::Document<'_, String>, ) -> Result<(), Box> { let schema_file_path = context_path.join(SCHEMA_FILE_NAME); @@ -139,7 +143,7 @@ async fn write_schema_file( fs::write(schema_file_path, schema_file).await?; Ok(()) } -async fn write_config_schema_file(context_path: &PathBuf) -> Result<(), Box> { +async fn write_config_schema_file(context_path: &Path) -> Result<(), Box> { let config_schema_file_path = context_path.join(CONFIG_SCHEMA_FILE_NAME); let config_schema_file = schema_for!(ServerConfigFile); fs::write( @@ -150,9 +154,7 @@ async fn write_config_schema_file(context_path: &PathBuf) -> Result<(), Box Result, Box> { +async fn read_config_file(context_path: &Path) -> Result, Box> { let file_path = context_path.join(CONFIG_FILE_NAME); let config: Option = match fs::read_to_string(file_path).await { Ok(file) => Some(serde_json::from_str(&file) @@ -165,7 +167,7 @@ async fn read_config_file( } async fn read_schema_file( - context_path: &PathBuf, + context_path: &Path, ) -> Result>, Box> { let file_path = context_path.join(SCHEMA_FILE_NAME); let config: Option> = match fs::read_to_string(file_path).await { @@ -178,21 +180,19 @@ async fn read_schema_file( Ok(config) } -async fn update_config(context_path: &PathBuf) -> Result<(), Box> { - let config_file = match read_config_file(&context_path).await? { +async fn update_config(context_path: &Path) -> Result<(), Box> { + let config_file = match read_config_file(context_path).await? { Some(config) => Ok(config), None => { println!("Configuration file {CONFIG_FILE_NAME} missing, initializing configuration directory."); - write_config_schema_file(&context_path).await?; - write_config_file(&context_path, &ServerConfigFile::default()).await?; + write_config_schema_file(context_path).await?; + write_config_file(context_path, &ServerConfigFile::default()).await?; Err::<_, String>("Configuration file could not be found, created a new one. Please fill in connection information before trying again.".into()) } }?; // CLI uses the introspection connection if available - let connection_file = config_file - .introspection - .unwrap_or_else(|| config_file.connection); + let connection_file = config_file.introspection.unwrap_or(config_file.connection); let connection = ConnectionConfig { endpoint: read_config_value(&connection_file.endpoint)?, @@ -200,7 +200,7 @@ async fn update_config(context_path: &PathBuf) -> Result<(), Box> { .headers .iter() .map(|(header_name, header_value)| { - Ok((header_name.to_owned(), read_config_value(&header_value)?)) + Ok((header_name.to_owned(), read_config_value(header_value)?)) }) .collect::>()?, }; @@ -213,7 +213,7 @@ async fn update_config(context_path: &PathBuf) -> Result<(), Box> { let schema = schema_from_introspection(introspection); write_schema_file(context_path, &schema).await?; - write_config_schema_file(&context_path).await?; + write_config_schema_file(context_path).await?; Ok(()) } @@ -228,7 +228,7 @@ fn read_config_value(value: &ConfigValue) -> Result #[tokio::test] #[ignore] async fn update_configuration_directory() { - update_config(&std::path::Path::new("../../config").to_path_buf()) + update_config(std::path::Path::new("../../config")) .await .expect("updating config should work"); } diff --git a/crates/ndc-graphql/src/connector.rs b/crates/ndc-graphql/src/connector.rs index 71ea8ea..f6d68cf 100644 --- a/crates/ndc-graphql/src/connector.rs +++ b/crates/ndc-graphql/src/connector.rs @@ -35,7 +35,7 @@ impl ConnectorSetup for GraphQLConnector { &self, configuration_dir: impl AsRef + Send, ) -> Result<::Configuration, ParseError> { - read_configuration(&configuration_dir.as_ref().to_path_buf()).await + read_configuration(configuration_dir.as_ref()).await } async fn try_init_state( @@ -43,7 +43,7 @@ impl ConnectorSetup for GraphQLConnector { configuration: &::Configuration, _metrics: &mut prometheus::Registry, ) -> Result<::State, InitializationError> { - Ok(ServerState::new(&configuration)) + Ok(ServerState::new(configuration)) } } @@ -100,7 +100,7 @@ impl Connector for GraphQLConnector { request: models::QueryRequest, ) -> Result, ExplainError> { let operation = tracing::info_span!("Build Query Document", internal.visibility = "user") - .in_scope(|| build_query_document(&request, &configuration))?; + .in_scope(|| build_query_document(&request, configuration))?; let query = serde_json::to_string_pretty(&GraphQLRequest::new( &operation.query, @@ -127,7 +127,7 @@ impl Connector for GraphQLConnector { ) -> Result, ExplainError> { let operation = tracing::info_span!("Build Mutation Document", internal.visibility = "user") - .in_scope(|| build_mutation_document(&request, &configuration))?; + .in_scope(|| build_mutation_document(&request, configuration))?; let query = serde_json::to_string_pretty(&GraphQLRequest::new( &operation.query, @@ -154,10 +154,10 @@ impl Connector for GraphQLConnector { ) -> Result, MutationError> { let operation = tracing::info_span!("Build Mutation Document", internal.visibility = "user") - .in_scope(|| build_mutation_document(&request, &configuration))?; + .in_scope(|| build_mutation_document(&request, configuration))?; let client = state - .client(&configuration) + .client(configuration) .await .map_err(|err| MutationError::Other(err.to_string().into()))?; @@ -180,8 +180,7 @@ impl Connector for GraphQLConnector { if let Some(errors) = response.errors { Err(MutationError::InvalidRequest( serde_json::to_string(&errors) - .map_err(|err| MutationError::Other(err.into()))? - .into(), + .map_err(|err| MutationError::Other(err.into()))?, )) } else if let Some(mut data) = response.data { let operation_results = request @@ -227,10 +226,10 @@ impl Connector for GraphQLConnector { request: models::QueryRequest, ) -> Result, QueryError> { let operation = tracing::info_span!("Build Query Document", internal.visibility = "user") - .in_scope(|| build_query_document(&request, &configuration))?; + .in_scope(|| build_query_document(&request, configuration))?; let client = state - .client(&configuration) + .client(configuration) .await .map_err(|err| QueryError::Other(err.to_string().into()))?; diff --git a/crates/ndc-graphql/src/connector/configuration.rs b/crates/ndc-graphql/src/connector/configuration.rs index ad74506..dda858b 100644 --- a/crates/ndc-graphql/src/connector/configuration.rs +++ b/crates/ndc-graphql/src/connector/configuration.rs @@ -1,4 +1,8 @@ -use std::{env, iter::once, path::PathBuf}; +use std::{ + env, + iter::once, + path::{Path, PathBuf}, +}; use common::{ config::{ConnectionConfig, ServerConfig}, @@ -9,11 +13,11 @@ use graphql_parser::parse_schema; use ndc_sdk::connector::{InvalidNode, InvalidNodes, KeyOrIndex, LocatedError, ParseError}; use tokio::fs; -pub async fn read_configuration(context_path: &PathBuf) -> Result { +pub async fn read_configuration(context_path: &Path) -> Result { let config_file_path = context_path.join(CONFIG_FILE_NAME); let config_file = fs::read_to_string(&config_file_path) .await - .map_err(|err| ParseError::IoError(err))?; + .map_err(ParseError::IoError)?; let config_file: ServerConfigFile = serde_json::from_str(&config_file).map_err(|err| { ParseError::ParseError(LocatedError { file_path: config_file_path.clone(), @@ -26,7 +30,7 @@ pub async fn read_configuration(context_path: &PathBuf) -> Result Result Result models::SchemaResponse { let mut functions = vec![]; for (name, field) in &configuration.schema.query_fields { - let response_type_name = configuration.response.query_response_type_name(&name); + let response_type_name = configuration.response.query_response_type_name(name); object_types.insert( response_type_name.clone(), @@ -142,7 +142,7 @@ pub fn schema_response(configuration: &ServerConfig) -> models::SchemaResponse { let mut procedures = vec![]; for (name, field) in &configuration.schema.mutation_fields { - let response_type_name = configuration.response.mutation_response_type_name(&name); + let response_type_name = configuration.response.mutation_response_type_name(name); object_types.insert( response_type_name.clone(), diff --git a/crates/ndc-graphql/src/query_builder.rs b/crates/ndc-graphql/src/query_builder.rs index 67fe7d4..29424cf 100644 --- a/crates/ndc-graphql/src/query_builder.rs +++ b/crates/ndc-graphql/src/query_builder.rs @@ -28,7 +28,7 @@ pub struct Operation { pub headers: BTreeMap, } -pub fn build_mutation_document<'a>( +pub fn build_mutation_document( request: &models::MutationRequest, configuration: &ServerConfig, ) -> Result { @@ -67,14 +67,14 @@ pub fn build_mutation_document<'a>( selection_set_field( &alias, - &name, + name, field_arguments( &procedure_arguments, |v| Ok(v.to_owned()), field_definition, &mut variables, )?, - &fields, + fields, field_definition, &mut variables, configuration, @@ -104,7 +104,7 @@ pub fn build_mutation_document<'a>( headers: request_headers, }) } -pub fn build_query_document<'a>( +pub fn build_query_document( request: &models::QueryRequest, configuration: &ServerConfig, ) -> Result { @@ -185,17 +185,14 @@ pub fn build_query_document<'a>( }) } +type Headers = BTreeMap; +type Arguments = BTreeMap; + fn extract_headers( arguments: &BTreeMap, map_argument: M, configuration: &ServerConfig, -) -> Result< - ( - BTreeMap, - BTreeMap, - ), - QueryBuilderError, -> +) -> Result<(Headers, Arguments), QueryBuilderError> where M: Fn(&A) -> Result, { @@ -203,7 +200,7 @@ where let mut headers = BTreeMap::new(); for (name, argument) in arguments { - let value = map_argument(&argument)?; + let value = map_argument(argument)?; if name == &configuration.request.headers_argument { match value { @@ -230,7 +227,7 @@ where } serde_json::Value::String(header) => { for pattern in &configuration.request.forward_headers { - if glob_match(&pattern, &name) { + if glob_match(pattern, &name) { headers.insert(name, header); break; } @@ -410,7 +407,7 @@ mod test { } "#; - let schema_document = graphql_parser::parse_schema(&schema_string)?; + let schema_document = graphql_parser::parse_schema(schema_string)?; let request_config = RequestConfig::default(); let response_config = ResponseConfig::default(); diff --git a/crates/ndc-graphql/src/query_builder/error.rs b/crates/ndc-graphql/src/query_builder/error.rs index 1fca98e..b216028 100644 --- a/crates/ndc-graphql/src/query_builder/error.rs +++ b/crates/ndc-graphql/src/query_builder/error.rs @@ -100,7 +100,7 @@ impl Display for QueryBuilderError { write!(f, "Field {field} not found in Mutation type") } QueryBuilderError::MisshapenHeadersArgument(headers) => { - write!(f, "Misshapen headers argument: {}", headers.to_string()) + write!(f, "Misshapen headers argument: {}", headers) } } } From 60a90206f96974bea6565c29e4cbeffddc007445 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Mon, 24 Jun 2024 19:49:46 -0400 Subject: [PATCH 3/5] Make config more explicit: require separate connection information --- crates/common/src/config_file.rs | 91 ++++++++++--------- crates/ndc-graphql-cli/src/main.rs | 48 ++++++---- .../src/connector/configuration.rs | 14 +-- 3 files changed, 82 insertions(+), 71 deletions(-) diff --git a/crates/common/src/config_file.rs b/crates/common/src/config_file.rs index 0256665..5e616d6 100644 --- a/crates/common/src/config_file.rs +++ b/crates/common/src/config_file.rs @@ -11,58 +11,23 @@ pub struct ServerConfigFile { #[serde(rename = "$schema")] pub json_schema: String, /// Connection configuration for query execution - /// Also used for introspection unless introspection connnection configuration is provided - pub connection: ConnectionConfigFile, + pub execution: ConnectionConfigFile, /// Optional Connection Configuration for introspection - pub introspection: Option, + pub introspection: ConnectionConfigFile, /// Optional configuration for requests - pub request: Option>>, + pub request: RequestConfig>, /// Optional configuration for responses - pub response: Option>>, + pub response: ResponseConfig>, } impl Default for ServerConfigFile { fn default() -> Self { Self { json_schema: CONFIG_SCHEMA_FILE_NAME.to_owned(), - connection: ConnectionConfigFile { - endpoint: ConfigValue::Value("".to_string()), - headers: BTreeMap::from_iter(vec![ - ( - "Content-Type".to_owned(), - ConfigValue::Value("application/json".to_string()), - ), - ( - "Authorization".to_owned(), - ConfigValue::ValueFromEnv("GRAPHQL_ENDPOINT_AUTHORIZATION".to_string()), - ), - ]), - }, - introspection: Some(ConnectionConfigFile { - endpoint: ConfigValue::Value("".to_string()), - headers: BTreeMap::from_iter(vec![ - ( - "Content-Type".to_owned(), - ConfigValue::Value("application/json".to_string()), - ), - ( - "Authorization".to_owned(), - ConfigValue::ValueFromEnv("GRAPHQL_ENDPOINT_AUTHORIZATION".to_string()), - ), - ]), - }), - request: Some(RequestConfig { - forward_headers: RequestConfig::default().forward_headers, - headers_argument: None, - headers_type_name: None, - }), - response: Some(ResponseConfig { - forward_headers: ResponseConfig::default().forward_headers, - headers_field: None, - response_field: None, - type_name_prefix: None, - type_name_suffix: None, - }), + execution: ConnectionConfigFile::default(), + introspection: ConnectionConfigFile::default(), + request: RequestConfig::default(), + response: ResponseConfig::default(), } } } @@ -74,6 +39,24 @@ pub struct ConnectionConfigFile { pub headers: BTreeMap, } +impl Default for ConnectionConfigFile { + fn default() -> Self { + Self { + endpoint: ConfigValue::Value("".to_string()), + headers: BTreeMap::from_iter(vec![ + ( + "Content-Type".to_owned(), + ConfigValue::Value("application/json".to_string()), + ), + ( + "Authorization".to_owned(), + ConfigValue::ValueFromEnv("GRAPHQL_ENDPOINT_AUTHORIZATION".to_string()), + ), + ]), + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] pub struct RequestConfig { @@ -131,6 +114,16 @@ impl Default for RequestConfig { } } +impl Default for RequestConfig> { + fn default() -> Self { + Self { + headers_argument: None, + headers_type_name: None, + forward_headers: vec!["*".to_owned()], + } + } +} + impl Default for ResponseConfig { fn default() -> Self { Self { @@ -143,6 +136,18 @@ impl Default for ResponseConfig { } } +impl Default for ResponseConfig> { + fn default() -> Self { + Self { + headers_field: None, + response_field: None, + type_name_prefix: None, + type_name_suffix: None, + forward_headers: vec!["*".to_owned()], + } + } +} + impl From>> for RequestConfig { fn from(value: RequestConfig>) -> Self { RequestConfig { diff --git a/crates/ndc-graphql-cli/src/main.rs b/crates/ndc-graphql-cli/src/main.rs index 1d7dc1c..712674f 100644 --- a/crates/ndc-graphql-cli/src/main.rs +++ b/crates/ndc-graphql-cli/src/main.rs @@ -95,7 +95,9 @@ async fn main() -> Result<(), Box> { println!("Configuration Initialized. Add your endpoint, then introspect your schema to continue.") } Command::Update {} => { - update_config(&context_path).await?; + let (config_file, schema_document) = update_config(&context_path).await?; + + validate_config(config_file, schema_document).await?; } Command::Validate {} => { let config_file = read_config_file(&context_path) @@ -105,17 +107,7 @@ async fn main() -> Result<(), Box> { .await? .ok_or_else(|| format!("Could not find {SCHEMA_FILE_NAME}"))?; - let request_config = config_file - .request - .map(|request| request.into()) - .unwrap_or_default(); - let response_config = config_file - .response - .map(|response| response.into()) - .unwrap_or_default(); - - let _schema = - SchemaDefinition::new(&schema_document, &request_config, &response_config)?; + validate_config(config_file, schema_document).await?; } Command::Watch {} => { todo!("implement watch command") @@ -180,7 +172,27 @@ async fn read_schema_file( Ok(config) } -async fn update_config(context_path: &Path) -> Result<(), Box> { +async fn validate_config( + config_file: ServerConfigFile, + schema_document: graphql_parser::schema::Document<'_, String>, +) -> Result<(), Box> { + let request_config = config_file.request.into(); + let response_config = config_file.response.into(); + + let _schema = SchemaDefinition::new(&schema_document, &request_config, &response_config)?; + + Ok(()) +} + +async fn update_config( + context_path: &Path, +) -> Result< + ( + ServerConfigFile, + graphql_parser::schema::Document<'static, String>, + ), + Box, +> { let config_file = match read_config_file(context_path).await? { Some(config) => Ok(config), None => { @@ -191,8 +203,8 @@ async fn update_config(context_path: &Path) -> Result<(), Box> { } }?; - // CLI uses the introspection connection if available - let connection_file = config_file.introspection.unwrap_or(config_file.connection); + // CLI uses the introspection connection + let connection_file = &config_file.introspection; let connection = ConnectionConfig { endpoint: read_config_value(&connection_file.endpoint)?, @@ -210,12 +222,12 @@ async fn update_config(context_path: &Path) -> Result<(), Box> { // todo: handle graphql errors! let introspection = response.data.expect("Successful introspection"); - let schema = schema_from_introspection(introspection); + let schema_document = schema_from_introspection(introspection); - write_schema_file(context_path, &schema).await?; + write_schema_file(context_path, &schema_document).await?; write_config_schema_file(context_path).await?; - Ok(()) + Ok((config_file, schema_document)) } fn read_config_value(value: &ConfigValue) -> Result { diff --git a/crates/ndc-graphql/src/connector/configuration.rs b/crates/ndc-graphql/src/connector/configuration.rs index dda858b..3b88c09 100644 --- a/crates/ndc-graphql/src/connector/configuration.rs +++ b/crates/ndc-graphql/src/connector/configuration.rs @@ -41,14 +41,8 @@ pub async fn read_configuration(context_path: &Path) -> Result Result Date: Tue, 25 Jun 2024 19:33:58 -0400 Subject: [PATCH 4/5] allow opting out of header forwarding behavior --- crates/common/src/config_file.rs | 28 ++- crates/ndc-graphql/src/connector.rs | 65 ++++--- crates/ndc-graphql/src/connector/schema.rs | 109 ++++++----- crates/ndc-graphql/src/query_builder.rs | 206 ++++++++++++--------- 4 files changed, 244 insertions(+), 164 deletions(-) diff --git a/crates/common/src/config_file.rs b/crates/common/src/config_file.rs index 5e616d6..7583a4f 100644 --- a/crates/common/src/config_file.rs +++ b/crates/common/src/config_file.rs @@ -71,7 +71,7 @@ pub struct RequestConfig { /// List of headers to from the request /// Defaults to ["*"], AKA all headers /// Supports glob patterns eg. "X-Hasura-*" - pub forward_headers: Vec, + pub forward_headers: Option>, } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "camelCase")] @@ -93,7 +93,7 @@ pub struct ResponseConfig { /// List of headers to from the response /// Defaults to ["*"], AKA all headers /// Supports glob patterns eg. "X-Hasura-*" - pub forward_headers: Vec, + pub forward_headers: Option>, } #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] @@ -109,7 +109,7 @@ impl Default for RequestConfig { Self { headers_argument: "_headers".to_owned(), headers_type_name: "_HeaderMap".to_owned(), - forward_headers: vec!["*".to_owned()], + forward_headers: Some(vec!["*".to_owned()]), } } } @@ -119,7 +119,7 @@ impl Default for RequestConfig> { Self { headers_argument: None, headers_type_name: None, - forward_headers: vec!["*".to_owned()], + forward_headers: Some(vec!["*".to_owned()]), } } } @@ -131,7 +131,7 @@ impl Default for ResponseConfig { response_field: "response".to_owned(), type_name_prefix: "_".to_owned(), type_name_suffix: "Response".to_owned(), - forward_headers: vec!["*".to_owned()], + forward_headers: Some(vec!["*".to_owned()]), } } } @@ -143,7 +143,7 @@ impl Default for ResponseConfig> { response_field: None, type_name_prefix: None, type_name_suffix: None, - forward_headers: vec!["*".to_owned()], + forward_headers: Some(vec!["*".to_owned()]), } } } @@ -157,7 +157,13 @@ impl From>> for RequestConfig { headers_type_name: value .headers_type_name .unwrap_or_else(|| Self::default().headers_type_name), - forward_headers: value.forward_headers, + forward_headers: value.forward_headers.and_then(|forward_headers| { + if forward_headers.is_empty() { + None + } else { + Some(forward_headers) + } + }), } } } @@ -176,7 +182,13 @@ impl From>> for ResponseConfig { type_name_suffix: value .type_name_suffix .unwrap_or_else(|| Self::default().type_name_suffix), - forward_headers: value.forward_headers, + forward_headers: value.forward_headers.and_then(|forward_headers| { + if forward_headers.is_empty() { + None + } else { + Some(forward_headers) + } + }), } } } diff --git a/crates/ndc-graphql/src/connector.rs b/crates/ndc-graphql/src/connector.rs index f6d68cf..a147363 100644 --- a/crates/ndc-graphql/src/connector.rs +++ b/crates/ndc-graphql/src/connector.rs @@ -164,13 +164,17 @@ impl Connector for GraphQLConnector { let execution_span = tracing::info_span!("Execute GraphQL Mutation", internal.visibility = "user"); - let (headers, response) = execute_graphql::( + let (headers, response) = execute_graphql::>( &operation.query, operation.variables, &configuration.connection.endpoint, &operation.headers, &client, - &configuration.response.forward_headers, + &configuration + .response + .forward_headers + .clone() + .unwrap_or_default(), ) .instrument(execution_span) .await @@ -183,6 +187,8 @@ impl Connector for GraphQLConnector { .map_err(|err| MutationError::Other(err.into()))?, )) } else if let Some(mut data) = response.data { + let forward_response_headers = configuration.response.forward_headers.is_some(); + let operation_results = request .operations .iter() @@ -191,19 +197,22 @@ impl Connector for GraphQLConnector { models::MutationOperation::Procedure { .. } => Ok({ let alias = format!("procedure_{index}"); let result = data - .get_mut(alias) + .get_mut(&alias) .map(|val| mem::replace(val, serde_json::Value::Null)) .unwrap_or(serde_json::Value::Null); - let response = BTreeMap::from_iter(vec![ - ( - configuration.response.headers_field.to_string(), - serde_json::to_value(&headers)?, - ), - (configuration.response.response_field.to_string(), result), - ]); - MutationOperationResults::Procedure { - result: serde_json::to_value(response)?, - } + let result = if forward_response_headers { + serde_json::to_value(BTreeMap::from_iter(vec![ + ( + configuration.response.headers_field.to_string(), + serde_json::to_value(&headers)?, + ), + (configuration.response.response_field.to_string(), result), + ]))? + } else { + result + }; + + MutationOperationResults::Procedure { result } }), }) .collect::, serde_json::Error>>() @@ -242,7 +251,11 @@ impl Connector for GraphQLConnector { &configuration.connection.endpoint, &operation.headers, &client, - &configuration.response.forward_headers, + &configuration + .response + .forward_headers + .clone() + .unwrap_or_default(), ) .instrument(execution_span) .await @@ -256,14 +269,15 @@ impl Connector for GraphQLConnector { .into(), )) } else if let Some(data) = response.data { - let headers = - serde_json::to_value(headers).map_err(|err| QueryError::Other(err.into()))?; - let data = - serde_json::to_value(data).map_err(|err| QueryError::Other(err.into()))?; + let forward_response_headers = configuration.response.forward_headers.is_some(); - Ok(JsonResponse::Value(models::QueryResponse(vec![RowSet { - aggregates: None, - rows: Some(vec![IndexMap::from_iter(vec![ + let row = if forward_response_headers { + let headers = serde_json::to_value(headers) + .map_err(|err| QueryError::Other(err.into()))?; + let data = + serde_json::to_value(data).map_err(|err| QueryError::Other(err.into()))?; + + IndexMap::from_iter(vec![ ( configuration.response.headers_field.to_string(), RowFieldValue(headers), @@ -272,7 +286,14 @@ impl Connector for GraphQLConnector { configuration.response.response_field.to_string(), RowFieldValue(data), ), - ])]), + ]) + } else { + data + }; + + Ok(JsonResponse::Value(models::QueryResponse(vec![RowSet { + aggregates: None, + rows: Some(vec![row]), }]))) } else { Err(QueryError::UnprocessableContent( diff --git a/crates/ndc-graphql/src/connector/schema.rs b/crates/ndc-graphql/src/connector/schema.rs index a46dc4c..bf36509 100644 --- a/crates/ndc-graphql/src/connector/schema.rs +++ b/crates/ndc-graphql/src/connector/schema.rs @@ -8,6 +8,9 @@ use ndc_sdk::models; use std::{collections::BTreeMap, iter}; pub fn schema_response(configuration: &ServerConfig) -> models::SchemaResponse { + let forward_request_headers = configuration.request.forward_headers.is_some(); + let forward_response_headers = configuration.response.forward_headers.is_some(); + let mut scalar_types: BTreeMap<_, _> = configuration .schema .definitions @@ -39,14 +42,16 @@ pub fn schema_response(configuration: &ServerConfig) -> models::SchemaResponse { }) .collect(); - scalar_types.insert( - configuration.request.headers_type_name.to_owned(), - models::ScalarType { - representation: Some(models::TypeRepresentation::JSON), - aggregate_functions: BTreeMap::new(), - comparison_operators: BTreeMap::new(), - }, - ); + if forward_request_headers { + scalar_types.insert( + configuration.request.headers_type_name.to_owned(), + models::ScalarType { + representation: Some(models::TypeRepresentation::JSON), + aggregate_functions: BTreeMap::new(), + comparison_operators: BTreeMap::new(), + }, + ); + } let mut object_types: BTreeMap<_, _> = configuration .schema @@ -109,20 +114,9 @@ pub fn schema_response(configuration: &ServerConfig) -> models::SchemaResponse { let mut functions = vec![]; for (name, field) in &configuration.schema.query_fields { - let response_type_name = configuration.response.query_response_type_name(name); - - object_types.insert( - response_type_name.clone(), - response_type(field, "function", name), - ); - - functions.push(models::FunctionInfo { - name: name.to_owned(), - description: field.description.to_owned(), - arguments: field - .arguments - .iter() - .map(map_argument) + let arguments = field.arguments.iter().map(map_argument); + let arguments = if forward_request_headers { + arguments .chain(iter::once(( configuration.request.headers_argument.to_owned(), models::ArgumentInfo { @@ -132,30 +126,40 @@ pub fn schema_response(configuration: &ServerConfig) -> models::SchemaResponse { }, }, ))) - .collect(), - result_type: models::Type::Named { + .collect() + } else { + arguments.collect() + }; + + let result_type = if forward_response_headers { + let response_type_name = configuration.response.query_response_type_name(name); + + object_types.insert( + response_type_name.clone(), + response_type(field, "function", name), + ); + + models::Type::Named { name: response_type_name, - }, + } + } else { + typeref_to_ndc_type(&field.r#type) + }; + + functions.push(models::FunctionInfo { + name: name.to_owned(), + description: field.description.to_owned(), + arguments, + result_type, }); } let mut procedures = vec![]; for (name, field) in &configuration.schema.mutation_fields { - let response_type_name = configuration.response.mutation_response_type_name(name); - - object_types.insert( - response_type_name.clone(), - response_type(field, "procedure", name), - ); - - procedures.push(models::ProcedureInfo { - name: name.to_owned(), - description: field.description.to_owned(), - arguments: field - .arguments - .iter() - .map(map_argument) + let arguments = field.arguments.iter().map(map_argument); + let arguments = if forward_request_headers { + arguments .chain(iter::once(( configuration.request.headers_argument.to_owned(), models::ArgumentInfo { @@ -165,10 +169,31 @@ pub fn schema_response(configuration: &ServerConfig) -> models::SchemaResponse { }, }, ))) - .collect(), - result_type: models::Type::Named { + .collect() + } else { + arguments.collect() + }; + + let result_type = if forward_response_headers { + let response_type_name = configuration.response.mutation_response_type_name(name); + + object_types.insert( + response_type_name.clone(), + response_type(field, "procedure", name), + ); + + models::Type::Named { name: response_type_name, - }, + } + } else { + typeref_to_ndc_type(&field.r#type) + }; + + procedures.push(models::ProcedureInfo { + name: name.to_owned(), + description: field.description.to_owned(), + arguments, + result_type, }); } diff --git a/crates/ndc-graphql/src/query_builder.rs b/crates/ndc-graphql/src/query_builder.rs index 29424cf..1e90db1 100644 --- a/crates/ndc-graphql/src/query_builder.rs +++ b/crates/ndc-graphql/src/query_builder.rs @@ -34,54 +34,56 @@ pub fn build_mutation_document( ) -> Result { let mut variables = OperationVariables::new(); - let mut request_headers = configuration.connection.headers.clone(); - - let selection_set = SelectionSet { - span: (pos(), pos()), - items: request - .operations - .iter() - .enumerate() - .map(|(index, operation)| match operation { - models::MutationOperation::Procedure { - name, - arguments, - fields, - } => { - let alias = format!("procedure_{index}"); - let field_definition = - configuration.schema.query_fields.get(name).ok_or_else(|| { - QueryBuilderError::QueryFieldNotFound { - field: name.to_owned(), - } - })?; + let mut request_headers = BTreeMap::new(); + let mut items = vec![]; + + for (index, operation) in request.operations.iter().enumerate() { + match operation { + models::MutationOperation::Procedure { + name, + arguments, + fields, + } => { + let alias = format!("procedure_{index}"); + let field_definition = + configuration.schema.query_fields.get(name).ok_or_else(|| { + QueryBuilderError::QueryFieldNotFound { + field: name.to_owned(), + } + })?; - let (headers, procedure_arguments) = - extract_headers(arguments, map_arg, configuration)?; + let (headers, procedure_arguments) = + extract_headers(arguments, map_arg, configuration)?; - for (name, header) in headers { - // if headers are duplicated, the last to be inserted stays - // todo: restrict what headers are forwarded here based on config - request_headers.insert(name, header.to_string()); - } + // note: duplicate headers get dropped here + // if there are multiple root fields, preset headers get set here once per field, + // with the last one persisting. + // this should not matter as headers should be identical anyways + request_headers.extend(headers.into_iter()); - selection_set_field( - &alias, - name, - field_arguments( - &procedure_arguments, - |v| Ok(v.to_owned()), - field_definition, - &mut variables, - )?, - fields, + let item = selection_set_field( + &alias, + name, + field_arguments( + &procedure_arguments, + |v| Ok(v.to_owned()), field_definition, &mut variables, - configuration, - ) - } - }) - .collect::>()?, + )?, + fields, + field_definition, + &mut variables, + configuration, + )?; + + items.push(item); + } + } + } + + let selection_set = SelectionSet { + span: (pos(), pos()), + items, }; let (values, variable_definitions) = variables.into_variable_definitions(); @@ -113,57 +115,69 @@ pub fn build_query_document( let (headers, request_arguments) = extract_headers(&request.arguments, map_query_arg, configuration)?; - // because all queries are commands, we can expect requests to always have this exact shape - let selection_set = SelectionSet { - span: (pos(), pos()), - items: request - .query - .fields - .as_ref() - .ok_or_else(|| QueryBuilderError::NoRequesQueryFields)? - .iter() - .map(|(alias, field)| { - let (fields, arguments) = match field { - models::Field::Column { - column, - fields, - arguments, - } if column == "__value" => Ok((fields, arguments)), - models::Field::Column { - column, - fields: _, - arguments: _, - } => Err(QueryBuilderError::NotSupported(format!( - "Expected field with key __value, got {column}" - ))), - models::Field::Relationship { .. } => { - Err(QueryBuilderError::NotSupported("Relationships".to_string())) - } - }?; + let mut items = vec![]; - if !arguments.is_empty() { - return Err(QueryBuilderError::Unexpected("Functions arguments should be passed to the collection, not the __value field".to_string())) - } + for (alias, field) in request + .query + .fields + .as_ref() + .ok_or_else(|| QueryBuilderError::NoRequesQueryFields)? + .iter() + { + let (fields, arguments) = match field { + models::Field::Column { + column, + fields, + arguments, + } if column == "__value" => Ok((fields, arguments)), + models::Field::Column { + column, + fields: _, + arguments: _, + } => Err(QueryBuilderError::NotSupported(format!( + "Expected field with key __value, got {column}" + ))), + models::Field::Relationship { .. } => { + Err(QueryBuilderError::NotSupported("Relationships".to_string())) + } + }?; - let field_definition = configuration.schema.query_fields.get(&request.collection).ok_or_else(|| QueryBuilderError::QueryFieldNotFound { field: request.collection.to_owned() })?; + if !arguments.is_empty() { + return Err(QueryBuilderError::Unexpected( + "Functions arguments should be passed to the collection, not the __value field" + .to_string(), + )); + } - selection_set_field( - alias, - &request.collection, - field_arguments( - &request_arguments, - map_arg, - field_definition, - &mut variables, + let field_definition = configuration + .schema + .query_fields + .get(&request.collection) + .ok_or_else(|| QueryBuilderError::QueryFieldNotFound { + field: request.collection.to_owned(), + })?; + + let item = selection_set_field( + alias, + &request.collection, + field_arguments( + &request_arguments, + map_arg, + field_definition, + &mut variables, + )?, + fields, + field_definition, + &mut variables, + configuration, + )?; + + items.push(item); + } - )?, - fields, - field_definition, - &mut variables, - configuration, - ) - }) - .collect::>()?, + let selection_set = SelectionSet { + span: (pos(), pos()), + items, }; let (values, variable_definitions) = variables.into_variable_definitions(); @@ -188,6 +202,8 @@ pub fn build_query_document( type Headers = BTreeMap; type Arguments = BTreeMap; +/// extract the headers argument if present and applicable +/// returns the headers for this request, including base headers and forwarded headers fn extract_headers( arguments: &BTreeMap, map_argument: M, @@ -197,7 +213,13 @@ where M: Fn(&A) -> Result, { let mut request_arguments = BTreeMap::new(); - let mut headers = BTreeMap::new(); + let mut headers = configuration.connection.headers.clone(); + + let patterns = configuration + .request + .forward_headers + .clone() + .unwrap_or_default(); for (name, argument) in arguments { let value = map_argument(argument)?; @@ -226,7 +248,7 @@ where )) } serde_json::Value::String(header) => { - for pattern in &configuration.request.forward_headers { + for pattern in &patterns { if glob_match(pattern, &name) { headers.insert(name, header); break; From 36412bfa6946c10cd3d45348791a13165da2fe98 Mon Sep 17 00:00:00 2001 From: Benoit Ranque Date: Thu, 27 Jun 2024 02:13:11 -0400 Subject: [PATCH 5/5] normalize patterns and headers to lowercase before comparison --- crates/common/src/client.rs | 2 +- crates/ndc-graphql/src/query_builder.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/common/src/client.rs b/crates/common/src/client.rs index 8ba97cc..db22fee 100644 --- a/crates/common/src/client.rs +++ b/crates/common/src/client.rs @@ -35,7 +35,7 @@ pub async fn execute_graphql( .iter() .filter_map(|(name, value)| { for pattern in return_headers { - if glob_match(pattern, name.as_str()) { + if glob_match(&pattern.to_lowercase(), &name.as_str().to_lowercase()) { return Some(( name.to_string(), value.to_str().unwrap_or_default().to_string(), diff --git a/crates/ndc-graphql/src/query_builder.rs b/crates/ndc-graphql/src/query_builder.rs index 1e90db1..5c96575 100644 --- a/crates/ndc-graphql/src/query_builder.rs +++ b/crates/ndc-graphql/src/query_builder.rs @@ -249,7 +249,7 @@ where } serde_json::Value::String(header) => { for pattern in &patterns { - if glob_match(pattern, &name) { + if glob_match(&pattern.to_lowercase(), &name.to_lowercase()) { headers.insert(name, header); break; }