graph, graphql: Handle selection/leaf mismatch

Make sure that we handle queries that have a selection from a scalar
field by ignoring the selection or that have no selection for a non-scalar
field by ignoring that field.

The latter differs from the previous behavior where the result would
contain an entry for such a field, but the data for the field would be an
empty object or a list of empty objects.

Author: lutter

graph/src/data/graphql/ext.rs

@@ -66,6 +66,13 @@ pub trait DocumentExt {
     fn get_named_type(&self, name: &str) -> Option<&TypeDefinition>;
 
     fn scalar_value_type(&self, field_type: &Type) -> ValueType;
+
+    /// Return `true` if the type does not allow selection of child fields.
+    ///
+    /// # Panics
+    ///
+    /// If `field_type` names an unknown type
+    fn is_leaf_type(&self, field_type: &Type) -> bool;
 }
 
 impl DocumentExt for Document {
@@ -219,6 +226,19 @@ impl DocumentExt for Document {
             Type::ListType(inner) => self.scalar_value_type(inner),
         }
     }
+
+    fn is_leaf_type(&self, field_type: &Type) -> bool {
+        match self
+            .get_named_type(field_type.get_base_type())
+            .expect("names of field types have been validated")
+        {
+            TypeDefinition::Enum(_) | TypeDefinition::Scalar(_) => true,
+            TypeDefinition::Object(_)
+            | TypeDefinition::Interface(_)
+            | TypeDefinition::Union(_)
+            | TypeDefinition::InputObject(_) => false,
+        }
+    }
 }
 
 pub trait TypeExt {

graphql/src/execution/query.rs

@@ -1,3 +1,4 @@
+use graph::data::graphql::DocumentExt as _;
 use graph::data::value::Object;
 use graphql_parser::Pos;
 use graphql_tools::validation::rules::*;
@@ -18,7 +19,7 @@ use graph::prelude::{info, o, q, r, s, BlockNumber, CheapClone, Logger, TryFromV
 
 use crate::execution::ast as a;
 use crate::query::{ast as qast, ext::BlockConstraint};
-use crate::schema::ast as sast;
+use crate::schema::ast::{self as sast};
 use crate::values::coercion;
 use crate::{execution::get_field, schema::api::ErrorPolicy};
 
@@ -861,21 +862,53 @@ impl Transform {
             selection_set,
         } = field;
 
+        // Short-circuit '__typename' since it is not a real field
+        if name == "__typename" {
+            return Ok(Some(a::Field {
+                position,
+                alias,
+                name,
+                arguments: vec![],
+                directives: vec![],
+                selection_set: a::SelectionSet::new(vec![]),
+            }));
+        }
+
+        let field_type = parent_type.field(&name).ok_or_else(|| {
+            vec![QueryExecutionError::UnknownField(
+                position,
+                parent_type.name().to_string(),
+                name.clone(),
+            )]
+        })?;
+
         let (directives, skip) = self.interpolate_directives(directives)?;
         if skip {
             return Ok(None);
         }
 
         let mut arguments = self.interpolate_arguments(arguments, &position)?;
         self.coerce_argument_values(&mut arguments, parent_type, &name)?;
+
+        let is_leaf_type = self.schema.document().is_leaf_type(&field_type.field_type);
         let selection_set = if selection_set.items.is_empty() {
+            if !is_leaf_type {
+                // see: graphql-bug-compat
+                // Field requires selection, ignore this field
+                return Ok(None);
+            }
             a::SelectionSet::new(vec![])
         } else {
-            let field_type = parent_type.field(&name).expect("field names are valid");
-            let ty = field_type.field_type.get_base_type();
-            let type_set = a::ObjectTypeSet::from_name(&self.schema, ty)?;
-            let ty = self.schema.object_or_interface(ty).unwrap();
-            self.expand_selection_set(selection_set, &type_set, ty)?
+            if is_leaf_type {
+                // see: graphql-bug-compat
+                // Field does not allow selections, ignore selections
+                a::SelectionSet::new(vec![])
+            } else {
+                let ty = field_type.field_type.get_base_type();
+                let type_set = a::ObjectTypeSet::from_name(&self.schema, ty)?;
+                let ty = self.schema.object_or_interface(ty).unwrap();
+                self.expand_selection_set(selection_set, &type_set, ty)?
+            }
         };
 
         Ok(Some(a::Field {

graphql/tests/query.rs

@@ -1452,6 +1452,41 @@ fn ignores_invalid_field_arguments() {
     })
 }
 
+// see: graphql-bug-compat
+#[test]
+fn leaf_selection_mismatch() {
+    run_test_sequentially(|store| async move {
+        let deployment = setup(store.as_ref());
+        let result = execute_query_document(
+            &deployment.hash,
+            // 'name' is a string and doesn't admit a selection
+            graphql_parser::parse_query("query { musician(id: \"m1\") { id name { wat }} } ")
+                .expect("invalid test query")
+                .into_static(),
+        )
+        .await;
+        let exp = object! {
+            musician: object! {
+                id: "m1",
+                name: "John"
+            }
+        };
+        let data = extract_data!(result).unwrap();
+        assert_eq!(exp, data);
+
+        let result = execute_query_document(
+            &deployment.hash,
+            // 'mainBand' is an object and requires a selection; it is ignored
+            graphql_parser::parse_query("query { musician(id: \"m1\") { id name mainBand } } ")
+                .expect("invalid test query")
+                .into_static(),
+        )
+        .await;
+        let data = extract_data!(result).unwrap();
+        assert_eq!(exp, data);
+    })
+}
+
 async fn check_musicians_at(
     id: &DeploymentHash,
     query: &str,