Skip to content

Commit

Permalink
feat: throw error in all resolvers for missing arguments (#952)
Browse files Browse the repository at this point in the history
  • Loading branch information
ologbonowiwi committed Jan 16, 2024
1 parent 728a8ef commit 56da067
Show file tree
Hide file tree
Showing 7 changed files with 231 additions and 130 deletions.
6 changes: 4 additions & 2 deletions src/blueprint/from_config/operators/graphql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ pub fn update_graphql<'a>(
operation_type: &'a GraphQLOperationType,
) -> TryFold<'a, (&'a Config, &'a Field, &'a config::Type, &'a str), FieldDefinition, String> {
TryFold::<(&Config, &Field, &config::Type, &'a str), FieldDefinition, String>::new(
|(config, field, _, _), b_field| {
|(config, field, type_of, _), b_field| {
let Some(graphql) = &field.graphql else {
return Valid::succeed(b_field);
};

compile_graphql(config, operation_type, graphql).map(|resolver| b_field.resolver(Some(resolver)))
compile_graphql(config, operation_type, graphql)
.map(|resolver| b_field.resolver(Some(resolver)))
.and_then(|b_field| b_field.validate_field(type_of, config).map_to(b_field))
},
)
}
3 changes: 2 additions & 1 deletion src/blueprint/from_config/operators/grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,14 @@ pub fn update_grpc<'a>(
operation_type: &'a GraphQLOperationType,
) -> TryFold<'a, (&'a Config, &'a Field, &'a config::Type, &'a str), FieldDefinition, String> {
TryFold::<(&Config, &Field, &config::Type, &'a str), FieldDefinition, String>::new(
|(config, field, _type_of, _name), b_field| {
|(config, field, type_of, _name), b_field| {
let Some(grpc) = &field.grpc else {
return Valid::succeed(b_field);
};

compile_grpc(CompileGrpc { config, operation_type, field, grpc, validate_with_schema: true })
.map(|resolver| b_field.resolver(Some(resolver)))
.and_then(|b_field| b_field.validate_field(type_of, config).map_to(b_field))
},
)
}
128 changes: 1 addition & 127 deletions src/blueprint/from_config/operators/http.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::blueprint::from_config::to_type;
use crate::blueprint::*;
use crate::config::group_by::GroupBy;
use crate::config::{Config, Field};
Expand All @@ -9,131 +8,6 @@ use crate::try_fold::TryFold;
use crate::valid::{Valid, ValidationError};
use crate::{config, helpers};

struct MustachePartsValidator<'a> {
type_of: &'a config::Type,
config: &'a Config,
field: &'a FieldDefinition,
}

impl<'a> MustachePartsValidator<'a> {
fn new(type_of: &'a config::Type, config: &'a Config, field: &'a FieldDefinition) -> Self {
Self { type_of, config, field }
}

fn validate_type(&self, parts: &[String], is_query: bool) -> Result<(), String> {
let mut len = parts.len();
let mut type_of = self.type_of;
for item in parts {
let field = type_of.fields.get(item).ok_or_else(|| {
format!(
"no value '{}' found",
parts[0..parts.len() - len + 1].join(".").as_str()
)
})?;
let val_type = to_type(field, None);

if !is_query && val_type.is_nullable() {
return Err(format!("value '{}' is a nullable type", item.as_str()));
} else if len == 1 && !is_scalar(val_type.name()) {
return Err(format!("value '{}' is not of a scalar type", item.as_str()));
} else if len == 1 {
break;
}

type_of = self
.config
.find_type(&field.type_of)
.ok_or_else(|| format!("no type '{}' found", parts.join(".").as_str()))?;

len -= 1;
}

Ok(())
}

fn validate(&self, parts: &[String], is_query: bool) -> Valid<(), String> {
let config = self.config;
let args = &self.field.args;

if parts.len() < 2 {
return Valid::fail("too few parts in template".to_string());
}

let head = parts[0].as_str();
let tail = parts[1].as_str();

match head {
"value" => {
// all items on parts except the first one
let tail = &parts[1..];

if let Err(e) = self.validate_type(tail, is_query) {
return Valid::fail(e);
}
}
"args" => {
// XXX this is a linear search but it's cost is less than that of
// constructing a HashMap since we'd have 3-4 arguments at max in
// most cases
if let Some(arg) = args.iter().find(|arg| arg.name == tail) {
if let Type::ListType { .. } = arg.of_type {
return Valid::fail(format!("can't use list type '{tail}' here"));
}

// we can use non-scalar types in args

if !is_query && arg.default_value.is_none() && arg.of_type.is_nullable() {
return Valid::fail(format!("argument '{tail}' is a nullable type"));
}
} else {
return Valid::fail(format!("no argument '{tail}' found"));
}
}
"vars" => {
if config.server.vars.get(tail).is_none() {
return Valid::fail(format!("var '{tail}' is not set in the server config"));
}
}
"headers" | "env" => {
// "headers" and "env" refers to values known at runtime, which we can't
// validate here
}
_ => {
return Valid::fail(format!("unknown template directive '{head}'"));
}
}

Valid::succeed(())
}
}

fn validate_field(type_of: &config::Type, config: &Config, field: &FieldDefinition) -> Valid<(), String> {
// XXX we could use `Mustache`'s `render` method with a mock
// struct implementing the `PathString` trait encapsulating `validation_map`
// but `render` simply falls back to the default value for a given
// type if it doesn't exist, so we wouldn't be able to get enough
// context from that method alone
// So we must duplicate some of that logic here :(

let parts_validator = MustachePartsValidator::new(type_of, config, field);

if let Some(Expression::Unsafe(Unsafe::Http { req_template, .. })) = &field.resolver {
Valid::from_iter(req_template.root_url.expression_segments(), |parts| {
parts_validator.validate(parts, false).trace("path")
})
.and(Valid::from_iter(req_template.query.clone(), |query| {
let (_, mustache) = query;

Valid::from_iter(mustache.expression_segments(), |parts| {
parts_validator.validate(parts, true).trace("query")
})
}))
.unit()
} else {
Valid::succeed(())
}
}

pub fn compile_http(config: &config::Config, field: &config::Field, http: &config::Http) -> Valid<Expression, String> {
Valid::<(), String>::fail("GroupBy is only supported for GET requests".to_string())
.when(|| !http.group_by.is_empty() && http.method != Method::GET)
Expand Down Expand Up @@ -189,7 +63,7 @@ pub fn update_http<'a>() -> TryFold<'a, (&'a Config, &'a Field, &'a config::Type

compile_http(config, field, http)
.map(|resolver| b_field.resolver(Some(resolver)))
.and_then(|b_field| validate_field(type_of, config, &b_field).map_to(b_field))
.and_then(|b_field| b_field.validate_field(type_of, config).map_to(b_field))
},
)
}
1 change: 1 addition & 0 deletions src/blueprint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ mod from_config;
mod into_schema;
mod operation;
mod timeout;
mod validate;

pub use blueprint::*;
pub use const_utils::*;
Expand Down
172 changes: 172 additions & 0 deletions src/blueprint/validate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
use super::{is_scalar, to_type, FieldDefinition, Type};
use crate::config::{self, Config};
use crate::lambda::{Expression, Unsafe};
use crate::valid::Valid;

struct MustachePartsValidator<'a> {
type_of: &'a config::Type,
config: &'a Config,
field: &'a FieldDefinition,
}

impl<'a> MustachePartsValidator<'a> {
fn new(type_of: &'a config::Type, config: &'a Config, field: &'a FieldDefinition) -> Self {
Self { type_of, config, field }
}

fn validate_type(&self, parts: &[String], is_query: bool) -> Result<(), String> {
let mut len = parts.len();
let mut type_of = self.type_of;
for item in parts {
let field = type_of.fields.get(item).ok_or_else(|| {
format!(
"no value '{}' found",
parts[0..parts.len() - len + 1].join(".").as_str()
)
})?;
let val_type = to_type(field, None);

if !is_query && val_type.is_nullable() {
return Err(format!("value '{}' is a nullable type", item.as_str()));
} else if len == 1 && !is_scalar(val_type.name()) {
return Err(format!("value '{}' is not of a scalar type", item.as_str()));
} else if len == 1 {
break;
}

type_of = self
.config
.find_type(&field.type_of)
.ok_or_else(|| format!("no type '{}' found", parts.join(".").as_str()))?;

len -= 1;
}

Ok(())
}

fn validate(&self, parts: &[String], is_query: bool) -> Valid<(), String> {
let config = self.config;
let args = &self.field.args;

if parts.len() < 2 {
return Valid::fail("too few parts in template".to_string());
}

let head = parts[0].as_str();
let tail = parts[1].as_str();

match head {
"value" => {
// all items on parts except the first one
let tail = &parts[1..];

if let Err(e) = self.validate_type(tail, is_query) {
return Valid::fail(e);
}
}
"args" => {
// XXX this is a linear search but it's cost is less than that of
// constructing a HashMap since we'd have 3-4 arguments at max in
// most cases
if let Some(arg) = args.iter().find(|arg| arg.name == tail) {
if let Type::ListType { .. } = arg.of_type {
return Valid::fail(format!("can't use list type '{tail}' here"));
}

// we can use non-scalar types in args
if !is_query && arg.default_value.is_none() && arg.of_type.is_nullable() {
return Valid::fail(format!("argument '{tail}' is a nullable type"));
}
} else {
return Valid::fail(format!("no argument '{tail}' found"));
}
}
"vars" => {
if config.server.vars.get(tail).is_none() {
return Valid::fail(format!("var '{tail}' is not set in the server config"));
}
}
"headers" | "env" => {
// "headers" and "env" refers to values known at runtime, which we can't
// validate here
}
_ => {
return Valid::fail(format!("unknown template directive '{head}'"));
}
}

Valid::succeed(())
}
}

impl FieldDefinition {
pub fn validate_field(&self, type_of: &config::Type, config: &Config) -> Valid<(), String> {
// XXX we could use `Mustache`'s `render` method with a mock
// struct implementing the `PathString` trait encapsulating `validation_map`
// but `render` simply falls back to the default value for a given
// type if it doesn't exist, so we wouldn't be able to get enough
// context from that method alone
// So we must duplicate some of that logic here :(
let parts_validator = MustachePartsValidator::new(type_of, config, self);

match &self.resolver {
Some(Expression::Unsafe(Unsafe::Http { req_template, .. })) => {
Valid::from_iter(req_template.root_url.expression_segments(), |parts| {
parts_validator.validate(parts, false).trace("path")
})
.and(Valid::from_iter(req_template.query.clone(), |query| {
let (_, mustache) = query;

Valid::from_iter(mustache.expression_segments(), |parts| {
parts_validator.validate(parts, true).trace("query")
})
}))
.unit()
}
Some(Expression::Unsafe(Unsafe::GraphQLEndpoint { req_template, .. })) => {
Valid::from_iter(req_template.headers.clone(), |(_, mustache)| {
Valid::from_iter(mustache.expression_segments(), |parts| {
parts_validator.validate(parts, true).trace("headers")
})
})
.and_then(|_| {
if let Some(args) = &req_template.operation_arguments {
Valid::from_iter(args, |(_, mustache)| {
Valid::from_iter(mustache.expression_segments(), |parts| {
parts_validator.validate(parts, true).trace("args")
})
})
} else {
Valid::succeed(Default::default())
}
})
.unit()
}
Some(Expression::Unsafe(Unsafe::Grpc { req_template, .. })) => {
Valid::from_iter(req_template.url.expression_segments(), |parts| {
parts_validator.validate(parts, false).trace("path")
})
.and(
Valid::from_iter(req_template.headers.clone(), |(_, mustache)| {
Valid::from_iter(mustache.expression_segments(), |parts| {
parts_validator.validate(parts, true).trace("headers")
})
})
.unit(),
)
.and_then(|_| {
if let Some(body) = &req_template.body {
Valid::from_iter(body.expression_segments(), |parts| {
parts_validator.validate(parts, true).trace("body")
})
} else {
Valid::succeed(Default::default())
}
})
.unit()
}
_ => Valid::succeed(()),
}
}
}
Loading

1 comment on commit 56da067

@github-actions
Copy link

Choose a reason for hiding this comment

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

Running 30s test @ http://localhost:8000/graphql

4 threads and 100 connections

Thread Stats Avg Stdev Max +/- Stdev
Latency 6.34ms 2.65ms 26.36ms 68.61%
Req/Sec 3.97k 104.13 4.76k 80.33%

474208 requests in 30.01s, 2.38GB read

Requests/sec: 15799.80

Transfer/sec: 81.10MB

Please sign in to comment.