Repository: apollographql/apollo-rs
Pull request #907 (2024-09-05 00:04:17)

fix: `adopt_orphan_extensions` + schema extensions + implicit root operation interaction
Building a schema with the `adopt_orphan_extensions` option enabled yields an error in the following simple case:
```graphql
extend schema { query: Query }

type Query { x: Int }
```

```
Error: duplicate definitions for the `query` root operation type
   ╭─[schema.graphql:2:25]
   │
 2 │         extend schema { query: Query }
   │                         ──────┬─────  
   │                               ╰─────── `query` redefined here
───╯
```

Currently, it accidentally makes the assumption that a schema extension's root operations do not use a default name, which explains why [this test](https://github.com/apollographql/apollo-rs/blob/caf820d86a03db3650149cefe6157442c73d7e50/crates/apollo-compiler/tests/extensions.rs#L5-L10) currently passes but the failing test from [f3f95b8](https://github.com/apollographql/apollo-rs/pull/907/commits/f3f95b8e65b4f4988843fd9151a7ffbfa70d9125) did not. This results in the `query` root operation type being added twice - once at the implicit step, and again when the orphan extensions are applied.

In the case that `adopt_orphan_extensions` is enabled, we need to scan the orphan schema extensions for root operations that match the default name and _skip_ the implicit addition step if so, since the extensions will be applied in a subsequent step.



Patch:
diff --git a/crates/apollo-compiler/src/schema/from_ast.rs b/crates/apollo-compiler/src/schema/from_ast.rs
index a226e3f..30c64eb 100644
--- a/crates/apollo-compiler/src/schema/from_ast.rs
+++ b/crates/apollo-compiler/src/schema/from_ast.rs
@@ -274,12 +274,28 @@ impl SchemaBuilder {
                 // This a macro rather than a closure to generate separate `static`s
                 let schema_def = schema.schema_definition.make_mut();
                 let mut has_implicit_root_operation = false;
-                for (operation_type, root_operation) in [
+                'root_operation_loop: for (operation_type, root_operation) in [
                     (OperationType::Query, &mut schema_def.query),
                     (OperationType::Mutation, &mut schema_def.mutation),
                     (OperationType::Subscription, &mut schema_def.subscription),
                 ] {
                     let name = operation_type.default_type_name();
+                    // If `adopt_orphan_extensions` is enabled, we should scan
+                    // each orphan schema extension for root operations. If we
+                    // see one, we should skip adding that particular implicit
+                    // root operation since the extensions will be applied
+                    // further down.
+                    if adopt_orphan_extensions {
+                        for ext in &orphan_extensions {
+                            for node in &ext.root_operations {
+                                let current_operation_type = node.as_ref().0;
+                                if current_operation_type == operation_type {
+                                    continue 'root_operation_loop;
+                                }
+                            }
+                        }
+                    }
+
                     if schema.types.get(&name).is_some_and(|def| def.is_object()) {
                         *root_operation = Some(name.into());
                         has_implicit_root_operation = true
diff --git a/crates/apollo-parser/src/parser/grammar/object.rs b/crates/apollo-parser/src/parser/grammar/object.rs
index 3d532fc..e4f029f 100644
--- a/crates/apollo-parser/src/parser/grammar/object.rs
+++ b/crates/apollo-parser/src/parser/grammar/object.rs
@@ -35,8 +35,6 @@ pub(crate) fn object_type_definition(p: &mut Parser) {
     if let Some(TokenKind::Name) = p.peek() {
         if p.peek_data().unwrap() == "implements" {
             implements_interfaces(p);
-        } else {
-            p.err("unexpected Name");
         }
     }
 


Test patch:
diff --git a/crates/apollo-compiler/tests/extensions.rs b/crates/apollo-compiler/tests/extensions.rs
index 897954f..d35ad17 100644
--- a/crates/apollo-compiler/tests/extensions.rs
+++ b/crates/apollo-compiler/tests/extensions.rs
@@ -34,6 +34,64 @@ fn test_orphan_extensions() {
     schema2.validate().unwrap();
 }
 
+#[test]
+fn test_orphan_extensions_schema_with_default_query_name() {
+    let input = r#"
+        extend schema { query: Query }
+        type Query { x: Int }
+    "#;
+
+    let schema = Schema::builder()
+        .adopt_orphan_extensions()
+        .parse(input, "schema.graphql")
+        .build()
+        .unwrap();
+
+    schema.validate().unwrap();
+}
+
+#[test]
+fn test_orphan_extensions_schema_def_with_extensions() {
+    let input = r#"
+        extend schema { query: Query }
+        extend schema { subscription: S }
+        schema { mutation: Mutation }
+        type Query { x: Int }
+        type Mutation { y: Int }
+        type S { z: Int }
+    "#;
+
+    let schema = Schema::builder()
+        .adopt_orphan_extensions()
+        .parse(input, "schema.graphql")
+        .build()
+        .unwrap();
+
+    schema.validate().unwrap();
+}
+
+#[test]
+fn test_invalid_orphan_extensions_schema_def_with_duplicate_root_operation() {
+    let input = r#"
+        extend schema { query: Query }
+        extend schema { subscription: S }
+        schema { mutation: Mutation, query: AnotherQuery }
+        type Query { x: Int }
+        type Mutation { y: Int }
+        type S { z: Int }
+        type AnotherQuery { a: String }
+    "#;
+
+    let invalid = Schema::builder()
+        .adopt_orphan_extensions()
+        .parse(input, "schema.graphql")
+        .build()
+        .unwrap_err();
+
+    let err = invalid.errors.to_string();
+    assert!(err.contains("duplicate definitions for the `query` root operation type"));
+}
+
 #[test]
 fn test_orphan_extensions_kind_mismatch() {
     let input = r#"
diff --git a/crates/apollo-parser/test_data/parser/ok/0042_object_type_definition_without_fields.graphql b/crates/apollo-parser/test_data/parser/ok/0042_object_type_definition_without_fields.graphql
new file mode 100644
index 0000000..5f06424
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/ok/0042_object_type_definition_without_fields.graphql
@@ -0,0 +1,6 @@
+"A type with no fields"
+type AnObjectTypeWithoutFields
+
+extend type AnObjectTypeWithoutFields {
+  id: ID!
+}
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/ok/0042_object_type_definition_without_fields.txt b/crates/apollo-parser/test_data/parser/ok/0042_object_type_definition_without_fields.txt
new file mode 100644
index 0000000..853507c
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/ok/0042_object_type_definition_without_fields.txt
@@ -0,0 +1,35 @@
+- DOCUMENT@0..107
+    - OBJECT_TYPE_DEFINITION@0..54
+        - DESCRIPTION@0..23
+            - STRING_VALUE@0..23
+                - STRING@0..23 "\"A type with no fields\""
+        - WHITESPACE@23..24 "\n"
+        - type_KW@24..28 "type"
+        - WHITESPACE@28..29 " "
+        - NAME@29..54
+            - IDENT@29..54 "AnObjectTypeWithoutFields"
+    - WHITESPACE@54..56 "\n\n"
+    - OBJECT_TYPE_EXTENSION@56..107
+        - extend_KW@56..62 "extend"
+        - WHITESPACE@62..63 " "
+        - type_KW@63..67 "type"
+        - WHITESPACE@67..68 " "
+        - NAME@68..93
+            - IDENT@68..93 "AnObjectTypeWithoutFields"
+        - WHITESPACE@93..94 " "
+        - FIELDS_DEFINITION@94..107
+            - L_CURLY@94..95 "{"
+            - WHITESPACE@95..98 "\n  "
+            - FIELD_DEFINITION@98..105
+                - NAME@98..100
+                    - IDENT@98..100 "id"
+                - COLON@100..101 ":"
+                - WHITESPACE@101..102 " "
+                - NON_NULL_TYPE@102..105
+                    - NAMED_TYPE@102..104
+                        - NAME@102..104
+                            - IDENT@102..104 "ID"
+                    - BANG@104..105 "!"
+            - WHITESPACE@105..106 "\n"
+            - R_CURLY@106..107 "}"
+recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/ok/0043_interface_type_definition_without_fields.graphql b/crates/apollo-parser/test_data/parser/ok/0043_interface_type_definition_without_fields.graphql
new file mode 100644
index 0000000..7714fec
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/ok/0043_interface_type_definition_without_fields.graphql
@@ -0,0 +1,6 @@
+"An interface with no fields"
+interface AnInterfaceWithoutFields
+
+extend interface AnInterfaceWithoutFields {
+  id: ID!
+}
diff --git a/crates/apollo-parser/test_data/parser/ok/0043_interface_type_definition_without_fields.txt b/crates/apollo-parser/test_data/parser/ok/0043_interface_type_definition_without_fields.txt
new file mode 100644
index 0000000..2f2048c
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/ok/0043_interface_type_definition_without_fields.txt
@@ -0,0 +1,36 @@
+- DOCUMENT@0..122
+    - INTERFACE_TYPE_DEFINITION@0..64
+        - DESCRIPTION@0..29
+            - STRING_VALUE@0..29
+                - STRING@0..29 "\"An interface with no fields\""
+        - WHITESPACE@29..30 "\n"
+        - interface_KW@30..39 "interface"
+        - WHITESPACE@39..40 " "
+        - NAME@40..64
+            - IDENT@40..64 "AnInterfaceWithoutFields"
+    - WHITESPACE@64..66 "\n\n"
+    - INTERFACE_TYPE_EXTENSION@66..121
+        - extend_KW@66..72 "extend"
+        - WHITESPACE@72..73 " "
+        - interface_KW@73..82 "interface"
+        - WHITESPACE@82..83 " "
+        - NAME@83..107
+            - IDENT@83..107 "AnInterfaceWithoutFields"
+        - WHITESPACE@107..108 " "
+        - FIELDS_DEFINITION@108..121
+            - L_CURLY@108..109 "{"
+            - WHITESPACE@109..112 "\n  "
+            - FIELD_DEFINITION@112..119
+                - NAME@112..114
+                    - IDENT@112..114 "id"
+                - COLON@114..115 ":"
+                - WHITESPACE@115..116 " "
+                - NON_NULL_TYPE@116..119
+                    - NAMED_TYPE@116..118
+                        - NAME@116..118
+                            - IDENT@116..118 "ID"
+                    - BANG@118..119 "!"
+            - WHITESPACE@119..120 "\n"
+            - R_CURLY@120..121 "}"
+    - WHITESPACE@121..122 "\n"
+recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/ok/0044_input_type_definition_without_input_values.graphql b/crates/apollo-parser/test_data/parser/ok/0044_input_type_definition_without_input_values.graphql
new file mode 100644
index 0000000..514929d
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/ok/0044_input_type_definition_without_input_values.graphql
@@ -0,0 +1,6 @@
+"An input with no input values"
+input AnInputWithoutInputValues
+
+extend input AnInputWithoutInputValues {
+  limit: Int!
+}
diff --git a/crates/apollo-parser/test_data/parser/ok/0044_input_type_definition_without_input_values.txt b/crates/apollo-parser/test_data/parser/ok/0044_input_type_definition_without_input_values.txt
new file mode 100644
index 0000000..4ecbf7b
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/ok/0044_input_type_definition_without_input_values.txt
@@ -0,0 +1,36 @@
+- DOCUMENT@0..122
+    - INPUT_OBJECT_TYPE_DEFINITION@0..63
+        - DESCRIPTION@0..31
+            - STRING_VALUE@0..31
+                - STRING@0..31 "\"An input with no input values\""
+        - WHITESPACE@31..32 "\n"
+        - input_KW@32..37 "input"
+        - WHITESPACE@37..38 " "
+        - NAME@38..63
+            - IDENT@38..63 "AnInputWithoutInputValues"
+    - WHITESPACE@63..65 "\n\n"
+    - INPUT_OBJECT_TYPE_EXTENSION@65..121
+        - extend_KW@65..71 "extend"
+        - WHITESPACE@71..72 " "
+        - input_KW@72..77 "input"
+        - WHITESPACE@77..78 " "
+        - NAME@78..103
+            - IDENT@78..103 "AnInputWithoutInputValues"
+        - WHITESPACE@103..104 " "
+        - INPUT_FIELDS_DEFINITION@104..121
+            - L_CURLY@104..105 "{"
+            - WHITESPACE@105..108 "\n  "
+            - INPUT_VALUE_DEFINITION@108..119
+                - NAME@108..113
+                    - IDENT@108..113 "limit"
+                - COLON@113..114 ":"
+                - WHITESPACE@114..115 " "
+                - NON_NULL_TYPE@115..119
+                    - NAMED_TYPE@115..118
+                        - NAME@115..118
+                            - IDENT@115..118 "Int"
+                    - BANG@118..119 "!"
+            - WHITESPACE@119..120 "\n"
+            - R_CURLY@120..121 "}"
+    - WHITESPACE@121..122 "\n"
+recursion limit: 500, high: 0
\ No newline at end of file