added a new validation phase

fixes

update to latest

run validations as soon as possible

use specified rules instead of default set

cleanup

fix fmt

added more rules

move validation check to Query struct
Added Leaf-Field-Selections rule

fixes

fixes

Added an option to disable graphql validations: `DISABLE_GRAPHQL_VALIDATIONS`

fixes

use more rules!

fixes

fix

cleanup

trigger ci

Author: dotansimha

Cargo.lock

@@ -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)) => {
+            assert_eq!(
+                error_message,
+                "Cannot query field \"parent\" on type \"Legged\"."
+            );
         }
         e => panic!("error {} is not the expected one", e),
     }
@@ -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.");
 
     let co_recursive = "
         query {
@@ -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\"."
+    );
 }

core/tests/interfaces.rs

@@ -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),
@@ -137,6 +138,7 @@ impl QueryExecutionError {
             | DeploymentReverted
             | SubgraphManifestResolveError(_)
             | InvalidSubgraphManifest
+            | ValidationError(_, _)
             | ResultTooBig(_, _) => false,
         }
     }
@@ -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")

graph/src/data/query/error.rs

@@ -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"

graphql/Cargo.toml

@@ -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;
@@ -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};
@@ -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 {}),
+                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,
@@ -115,6 +152,24 @@ 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.clone().into_iter().nth(0),
+                        e.message,
+                    )
+                })
+                .collect());
+        }
+
         let mut operation = None;
         let mut fragments = HashMap::new();
         for defn in query.document.definitions.into_iter() {

graphql/src/execution/query.rs

@@ -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
+                      }
+                  }
                 }
             }",
             )
@@ -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(3030100, _) => (), // Expected
+                e => panic!("did not catch complexity: {:?}", e),
             },
             _ => panic!("did not catch complexity"),
         }

graphql/tests/query.rs

@@ -13,4 +13,4 @@ graphql-parser = "0.4.0"
 http = "0.2"
 hyper = "0.14"
 serde = "1.0"
-either = "1.6.1"
+either = "1.6.1"

server/index-node/Cargo.toml

@@ -15,4 +15,4 @@ graph-store-postgres = { path = "../postgres" }
 lazy_static = "1.1"
 hex-literal = "0.3"
 diesel = { version = "1.4.8", features = ["postgres", "serde_json", "numeric", "r2d2"] }
-graph-chain-ethereum = { path = "../../chain/ethereum" }
+graph-chain-ethereum = { path = "../../chain/ethereum" }