Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

graph, graphql: introduce GraphQL spec-compliant validation phase and rules #3057

Merged
merged 1 commit into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Cargo.lock

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

15 changes: 10 additions & 5 deletions core/tests/interfaces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,11 @@ async fn follow_interface_reference_invalid() {
.unwrap();

match &res.to_result().unwrap_err()[0] {
QueryError::ExecutionError(QueryExecutionError::UnknownField(_, type_name, field_name)) => {
assert_eq!(type_name, "Legged");
assert_eq!(field_name, "parent");
QueryError::ExecutionError(QueryExecutionError::ValidationError(_, error_message)) => {
dotansimha marked this conversation as resolved.
Show resolved Hide resolved
assert_eq!(
error_message,
"Cannot query field \"parent\" on type \"Legged\"."
);
}
e => panic!("error {} is not the expected one", e),
}
Expand Down Expand Up @@ -1424,7 +1426,7 @@ async fn recursive_fragment() {
.await
.unwrap();
let data = res.to_result().unwrap_err()[0].to_string();
assert_eq!(data, "query has fragment cycle including `FooFrag`");
assert_eq!(data, "Cannot spread fragment \"FooFrag\" within itself.");
dotansimha marked this conversation as resolved.
Show resolved Hide resolved

let co_recursive = "
query {
Expand All @@ -1451,5 +1453,8 @@ async fn recursive_fragment() {
.await
.unwrap();
let data = res.to_result().unwrap_err()[0].to_string();
assert_eq!(data, "query has fragment cycle including `BarFrag`");
assert_eq!(
data,
"Cannot spread fragment \"BarFrag\" within itself via \"FooFrag\"."
dotansimha marked this conversation as resolved.
Show resolved Hide resolved
);
}
5 changes: 5 additions & 0 deletions graph/src/data/query/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub enum QueryExecutionError {
AbstractTypeError(String),
InvalidArgumentError(Pos, String, q::Value),
MissingArgumentError(Pos, String),
ValidationError(Option<Pos>, String),
InvalidVariableTypeError(Pos, String),
MissingVariableError(Pos, String),
ResolveEntitiesError(String),
Expand Down Expand Up @@ -137,6 +138,7 @@ impl QueryExecutionError {
| DeploymentReverted
| SubgraphManifestResolveError(_)
| InvalidSubgraphManifest
| ValidationError(_, _)
| ResultTooBig(_, _) => false,
}
}
Expand All @@ -161,6 +163,9 @@ impl fmt::Display for QueryExecutionError {
OperationNotFound(s) => {
write!(f, "Operation name not found `{}`", s)
}
ValidationError(_pos, message) => {
write!(f, "{}", message)
}
NotSupported(s) => write!(f, "Not supported: {}", s),
NoRootSubscriptionObjectType => {
write!(f, "No root Subscription type defined in the schema")
Expand Down
1 change: 1 addition & 0 deletions graphql/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ crossbeam = "0.8"
futures01 = { package="futures", version="0.1.29" }
graph = { path = "../graph" }
graphql-parser = "0.4.0"
graphql-tools = "0.0.15"
indexmap = "1.7"
Inflector = "0.11.3"
lazy_static = "1.2.0"
Expand Down
58 changes: 55 additions & 3 deletions graphql/src/execution/query.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use graphql_parser::Pos;
use graphql_tools::validation::rules::*;
use graphql_tools::validation::validate::{validate, ValidationPlan};
use lazy_static::lazy_static;
use std::collections::{HashMap, HashSet};
use std::hash::{Hash, Hasher};
use std::sync::Arc;
Expand All @@ -9,11 +12,10 @@ use graph::data::graphql::{
ext::{DocumentExt, TypeExt},
ObjectOrInterface,
};
use graph::data::query::QueryExecutionError;
use graph::data::query::{Query as GraphDataQuery, QueryVariables};
use graph::data::schema::ApiSchema;
use graph::prelude::{
info, o, q, r, s, BlockNumber, CheapClone, Logger, QueryExecutionError, TryFromValue,
};
use graph::prelude::{info, o, q, r, s, BlockNumber, CheapClone, Logger, TryFromValue};

use crate::introspection::introspection_schema;
use crate::query::{ast as qast, ext::BlockConstraint};
Expand All @@ -23,6 +25,41 @@ use crate::{
schema::api::ErrorPolicy,
};

lazy_static! {
static ref GRAPHQL_VALIDATION_PLAN: ValidationPlan = ValidationPlan::from(
if std::env::var("DISABLE_GRAPHQL_VALIDATIONS")
.unwrap_or_else(|_| "false".into())
.parse::<bool>()
.unwrap_or_else(|_| false)
{
vec![]
} else {
vec![
Box::new(UniqueOperationNames {}),
Box::new(LoneAnonymousOperation {}),
Box::new(SingleFieldSubscriptions {}),
Box::new(KnownTypeNames {}),
Box::new(FragmentsOnCompositeTypes {}),
Box::new(VariablesAreInputTypes {}),
Box::new(LeafFieldSelections {}),
Box::new(FieldsOnCorrectType {}),
Box::new(UniqueFragmentNames {}),
Box::new(KnownFragmentNames {}),
Box::new(NoUnusedFragments {}),
Box::new(OverlappingFieldsCanBeMerged {}),
Box::new(NoFragmentsCycle {}),
dotansimha marked this conversation as resolved.
Show resolved Hide resolved
Box::new(PossibleFragmentSpreads {}),
Box::new(NoUnusedVariables {}),
Box::new(NoUndefinedVariables {}),
Box::new(KnownArgumentNames {}),
Box::new(UniqueArgumentNames {}),
Box::new(UniqueVariableNames {}),
Box::new(ProvidedRequiredArguments {}),
]
}
);
}

#[derive(Clone, Debug)]
pub enum ComplexityError {
TooDeep,
Expand Down Expand Up @@ -115,6 +152,21 @@ impl Query {
max_complexity: Option<u64>,
max_depth: u8,
) -> Result<Arc<Self>, Vec<QueryExecutionError>> {
let validation_errors = validate(
&schema.document(),
&query.document,
&GRAPHQL_VALIDATION_PLAN,
);

if validation_errors.len() > 0 {
return Err(validation_errors
.into_iter()
.map(|e| {
QueryExecutionError::ValidationError(e.locations.first().cloned(), e.message)
})
.collect());
}

let mut operation = None;
let mut fragments = HashMap::new();
for defn in query.document.definitions.into_iter() {
Expand Down
21 changes: 11 additions & 10 deletions graphql/tests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,17 +895,18 @@ fn query_complexity_subscriptions() {
"subscription {
musicians(orderBy: id) {
name
bands(first: 100, orderBy: id) {
t1: bands(first: 100, orderBy: id) {
name
members(first: 100, orderBy: id) {
name
}
}
}
__schema {
types {
name
}
t2: bands(first: 200, orderBy: id) {
name
members(first: 100, orderBy: id) {
name
}
}
dotansimha marked this conversation as resolved.
Show resolved Hide resolved
}
}",
)
Expand All @@ -932,12 +933,12 @@ fn query_complexity_subscriptions() {
result_size: result_size_metrics(),
};

// The extra introspection causes the complexity to go over.
let result = execute_subscription(Subscription { query }, schema, options);

match result {
Err(SubscriptionError::GraphQLError(e)) => match e[0] {
QueryExecutionError::TooComplex(1_010_200, _) => (), // Expected
_ => panic!("did not catch complexity"),
Err(SubscriptionError::GraphQLError(e)) => match &e[0] {
QueryExecutionError::TooComplex(3_030_100, _) => (), // Expected
e => panic!("did not catch complexity: {:?}", e),
},
_ => panic!("did not catch complexity"),
}
Expand Down