feat(compiler): validate field merging using the Xing algorithm

crates/apollo-compiler/CHANGELOG.md

@@ -17,6 +17,21 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ## Maintenance
 ## Documentation-->
 
+# [x.x.x] (unreleased) - 2024-mm-dd
+
+## Features
+- **New field merging validation implementation - [goto-bus-stop], [pull/816]**
+  - Uses the much more scalable [Xing algorithm].
+  - This also fixes some invalid selections that were previously accepted by apollo-compiler.
+  - Selections in fragment definitions are now only validated in the context of the operations they
+    are used in, reducing duplicate error reports for invalid fragments. This means invalid *unused*
+    fragments do not produce field merging errors, but they will still raise a different error because
+    fragments being unused is also an error according to the spec.
+
+[goto-bus-stop]: https://github.com/goto-bus-stop]
+[pull/816]: https://github.com/apollographql/apollo-rs/pull/816
+[Xing algorithm]: https://tech.new-work.se/graphql-overlapping-fields-can-be-merged-fast-ea6e92e0a01
+
 # [1.0.0-beta.12](https://crates.io/crates/apollo-compiler/1.0.0-beta.12) - 2024-01-15
 
 ## BREAKING

crates/apollo-compiler/Cargo.toml

@@ -49,6 +49,11 @@ name = "multi-source"
 path = "benches/multi_source.rs"
 harness = false
 
+[[bench]]
+name = "fields-validation"
+path = "benches/fields_validation.rs"
+harness = false
+
 [[test]]
 name = "main"
 

crates/apollo-compiler/benches/fields_validation.rs

@@ -0,0 +1,124 @@
+use apollo_compiler::ExecutableDocument;
+use apollo_compiler::Schema;
+use criterion::*;
+
+fn bench_many_same_field(c: &mut Criterion) {
+    let schema =
+        Schema::parse_and_validate("type Query { hello: String! }", "schema.graphql").unwrap();
+    let query = format!("{{ {} }}", "hello ".repeat(1_000));
+
+    c.bench_function("many_same_field", move |b| {
+        b.iter(|| {
+            let doc =
+                ExecutableDocument::parse_and_validate(&schema, &query, "query.graphql").unwrap();
+            black_box(doc);
+        });
+    });
+}
+
+fn bench_many_same_nested_field(c: &mut Criterion) {
+    let schema = Schema::parse_and_validate(
+        "
+        type Nested { hello: String! }
+        type Query { nested: Nested! }
+    ",
+        "schema.graphql",
+    )
+    .unwrap();
+    let query = format!("{{ {} }}", "nested { hello } ".repeat(1_000));
+
+    c.bench_function("many_same_nested_field", move |b| {
+        b.iter(|| {
+            let doc =
+                ExecutableDocument::parse_and_validate(&schema, &query, "query.graphql").unwrap();
+            black_box(doc);
+        });
+    });
+}
+
+fn bench_many_arguments(c: &mut Criterion) {
+    let schema =
+        Schema::parse_and_validate("type Query { hello: String! }", "schema.graphql").unwrap();
+    let args = (0..2_000).fold(String::new(), |mut acc, i| {
+        use std::fmt::Write;
+        _ = writeln!(&mut acc, "arg{i}: {i}");
+        acc
+    });
+    let field = format!("hello({args})");
+    let query = format!("{{ {field} {field} }}");
+
+    c.bench_function("many_arguments", move |b| {
+        b.iter(|| {
+            // Will return errors but that's cool
+            let doc = ExecutableDocument::parse_and_validate(&schema, &query, "query.graphql");
+            _ = black_box(doc);
+        });
+    });
+}
+
+fn bench_many_types(c: &mut Criterion) {
+    let schema = Schema::parse_and_validate(
+        "
+        interface Abstract {
+          field: Abstract
+          leaf: Int
+        }
+        interface Abstract1 {
+          field: Abstract
+          leaf: Int
+        }
+        interface Abstract2 {
+          field: Abstract
+          leaf: Int
+        }
+        type Concrete1 implements Abstract & Abstract1 {
+          field: Abstract
+          leaf: Int
+        }
+        type Concrete2 implements Abstract & Abstract2 {
+          field: Abstract
+          leaf: Int
+        }
+        type Query {
+            field: Abstract
+        }
+    ",
+        "schema.graphql",
+    )
+    .unwrap();
+    let query = format!(
+        "
+        fragment multiply on Abstract {{
+           field {{
+             ... on Abstract1 {{ field {{ leaf }} }}
+             ... on Abstract2 {{ field {{ leaf }} }}
+             ... on Concrete1 {{ field {{ leaf }} }}
+             ... on Concrete2 {{ field {{ leaf }} }}
+           }}
+        }}
+
+        query DeepAbstractConcrete {{
+            {open}{close}
+        }}
+    ",
+        open = "field { ...multiply ".repeat(100),
+        close = "}".repeat(100)
+    );
+
+    c.bench_function("many_types", move |b| {
+        b.iter(|| {
+            let doc =
+                ExecutableDocument::parse_and_validate(&schema, &query, "query.graphql").unwrap();
+            black_box(doc);
+        });
+    });
+}
+
+criterion_group!(
+    fields,
+    bench_many_same_field,
+    bench_many_same_nested_field,
+    bench_many_arguments,
+    bench_many_types,
+);
+criterion_main!(fields);

crates/apollo-compiler/benches/testdata/supergraph_query.graphql

@@ -1,10 +1,5 @@
 query Query {
-  allProducts {
-    id
+  topProducts {
     sku
-    createdBy {
-      email
-      totalProductsCreated
-    }
   }
-}
+}

crates/apollo-compiler/src/executable/mod.rs

@@ -2,6 +2,8 @@
 //! which can contain operations and fragments.
 
 use crate::ast;
+use crate::coordinate::FieldArgumentCoordinate;
+use crate::coordinate::TypeAttributeCoordinate;
 use crate::schema;
 use crate::Node;
 use crate::Parser;
@@ -69,20 +71,20 @@ pub struct Fragment {
     pub selection_set: SelectionSet,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct SelectionSet {
     pub ty: NamedType,
     pub selections: Vec<Selection>,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum Selection {
     Field(Node<Field>),
     FragmentSpread(Node<FragmentSpread>),
     InlineFragment(Node<InlineFragment>),
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Field {
     /// The definition of this field in an object type or interface type definition in the schema
     pub definition: Node<schema::FieldDefinition>,
@@ -93,20 +95,21 @@ pub struct Field {
     pub selection_set: SelectionSet,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct FragmentSpread {
     pub fragment_name: Name,
     pub directives: DirectiveList,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct InlineFragment {
     pub type_condition: Option<NamedType>,
     pub directives: DirectiveList,
     pub selection_set: SelectionSet,
 }
 
-/// AST node that has been skipped during conversion to `ExecutableDocument`
+/// Errors that can occur during conversion from AST to executable document or
+/// validation of an executable document.
 #[derive(thiserror::Error, Debug, Clone)]
 pub(crate) enum BuildError {
     #[error("an executable document must not contain {describe}")]
@@ -161,6 +164,67 @@ pub(crate) enum BuildError {
         field_name: Name,
         path: SelectionPath,
     },
+
+    // Validation errors
+    #[error(
+        "{} can only have one root field",
+        subscription_name_or_anonymous(name)
+    )]
+    SubscriptionUsesMultipleFields {
+        name: Option<Name>,
+        fields: Vec<Name>,
+    },
+
+    #[error(
+        "{} can not have an introspection field as a root field",
+        subscription_name_or_anonymous(name)
+    )]
+    SubscriptionUsesIntrospection {
+        /// Name of the operation
+        name: Option<Name>,
+        /// Name of the introspection field
+        field: Name,
+    },
+
+    #[error("operation must not select different types using the same name `{alias}`")]
+    ConflictingFieldType {
+        /// Name or alias of the non-unique field.
+        alias: Name,
+        original_location: Option<NodeLocation>,
+        original_coordinate: TypeAttributeCoordinate,
+        original_type: Type,
+        conflicting_location: Option<NodeLocation>,
+        conflicting_coordinate: TypeAttributeCoordinate,
+        conflicting_type: Type,
+    },
+    #[error("operation must not provide conflicting field arguments for the same name `{alias}`")]
+    ConflictingFieldArgument {
+        /// Name or alias of the non-unique field.
+        alias: Name,
+        original_location: Option<NodeLocation>,
+        original_coordinate: FieldArgumentCoordinate,
+        original_value: Option<Value>,
+        conflicting_location: Option<NodeLocation>,
+        conflicting_coordinate: FieldArgumentCoordinate,
+        conflicting_value: Option<Value>,
+    },
+    #[error("cannot select different fields into the same alias `{alias}`")]
+    ConflictingFieldName {
+        /// Name of the non-unique field.
+        alias: Name,
+        original_location: Option<NodeLocation>,
+        original_selection: TypeAttributeCoordinate,
+        conflicting_location: Option<NodeLocation>,
+        conflicting_selection: TypeAttributeCoordinate,
+    },
+}
+
+fn subscription_name_or_anonymous(name: &Option<Name>) -> impl std::fmt::Display + '_ {
+    crate::validation::diagnostics::NameOrAnon {
+        name: name.as_ref(),
+        if_some_prefix: "subscription",
+        if_none: "anonymous subscription",
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -327,10 +391,26 @@ impl PartialEq for ExecutableDocument {
 }
 
 impl Operation {
+    /// Returns the name of the schema type this operation selects against.
     pub fn object_type(&self) -> &NamedType {
         &self.selection_set.ty
     }
 
+    /// Returns true if this is a query operation.
+    pub fn is_query(&self) -> bool {
+        self.operation_type == OperationType::Query
+    }
+
+    /// Returns true if this is a mutation operation.
+    pub fn is_mutation(&self) -> bool {
+        self.operation_type == OperationType::Mutation
+    }
+
+    /// Returns true if this is a subscription operation.
+    pub fn is_subscription(&self) -> bool {
+        self.operation_type == OperationType::Subscription
+    }
+
     /// Return whether this operation is a query that only selects introspection meta-fields:
     /// `__type`, `__schema`, and `__typename`
     pub fn is_introspection(&self, document: &ExecutableDocument) -> bool {
@@ -591,6 +671,11 @@ impl Field {
         schema.types.get(self.ty().inner_named_type())
     }
 
+    /// Returns the argument by a given name.
+    pub fn argument_by_name(&self, name: &str) -> Option<&'_ Node<Argument>> {
+        self.arguments.iter().find(|argument| argument.name == name)
+    }
+
     serialize_method!();
 }
 

crates/apollo-compiler/src/executable/validation.rs

@@ -1,5 +1,6 @@
 use super::FieldSet;
 use crate::ast;
+use crate::validation::selection::FieldsInSetCanMerge;
 use crate::validation::Details;
 use crate::validation::DiagnosticList;
 use crate::validation::FileId;
@@ -30,11 +31,15 @@ pub(crate) fn validate_standalone_executable(
 }
 
 fn validate_with_schema(
-    _errors: &mut DiagnosticList,
-    _schema: &Schema,
-    _document: &ExecutableDocument,
+    errors: &mut DiagnosticList,
+    schema: &Schema,
+    document: &ExecutableDocument,
 ) {
-    // TODO
+    let mut fields_in_set_can_merge = FieldsInSetCanMerge::new(schema, document);
+    for operation in document.all_operations() {
+        crate::validation::operation::validate_subscription(document, operation, errors);
+        fields_in_set_can_merge.validate_operation(operation, errors);
+    }
 }
 
 pub(crate) fn validate_with_or_without_schema(