fix: include mutation root type definition in connectors (#6516)

apollo-federation/cli/src/main.rs

@@ -14,6 +14,7 @@ use apollo_federation::query_plan::query_planner::QueryPlannerConfig;
 use apollo_federation::sources::connect::expand::expand_connectors;
 use apollo_federation::sources::connect::expand::ExpansionResult;
 use apollo_federation::subgraph;
+use apollo_federation::ApiSchemaOptions;
 use apollo_federation::Supergraph;
 use bench::BenchOutput;
 use clap::Parser;
@@ -323,8 +324,10 @@ fn cmd_expand(
     filter_prefix: Option<&str>,
 ) -> Result<(), FederationError> {
     let original_supergraph = load_supergraph_file(file_path)?;
-    let ExpansionResult::Expanded { raw_sdl, .. } =
-        expand_connectors(&original_supergraph.schema.schema().serialize().to_string())?
+    let ExpansionResult::Expanded { raw_sdl, .. } = expand_connectors(
+        &original_supergraph.schema.schema().serialize().to_string(),
+        &ApiSchemaOptions::default(),
+    )?
     else {
         return Err(FederationError::internal(
             "supplied supergraph has no connectors to expand",

apollo-federation/src/merge/snapshots/apollo_federation__merge__tests__basic.snap

@@ -1,9 +1,10 @@
 ---
-source: apollo-federation/src/merge.rs
+source: apollo-federation/src/merge/tests.rs
 expression: schema.serialize()
 ---
 schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) {
   query: Query
+  mutation: Mutation
 }
 
 directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
@@ -86,3 +87,12 @@ type Y @join__type(graph: BASIC_1) @join__type(graph: BASIC_2) {
 input YInput @join__type(graph: BASIC_1) @join__type(graph: BASIC_2) {
   z: ID @join__field(graph: BASIC_1, type: "ID") @join__field(graph: BASIC_2, type: "ID")
 }
+
+type Mutation @join__type(graph: BASIC_1) @join__type(graph: BASIC_2) {
+  m: M @join__field(graph: BASIC_1, type: "M")
+  m2(x: ID, y: YInput): M @join__field(graph: BASIC_2, type: "M")
+}
+
+type M @join__type(graph: BASIC_1) @join__type(graph: BASIC_2) {
+  n: String @join__field(graph: BASIC_1, type: "String") @join__field(graph: BASIC_2, type: "String")
+}

apollo-federation/src/sources/connect/expand/merge/basic_1.graphql

@@ -39,3 +39,11 @@ type Y {
 input YInput {
   z: ID
 }
+
+type Mutation {
+  m: M
+}
+
+type M {
+  n: String
+}

apollo-federation/src/sources/connect/expand/merge/basic_2.graphql

@@ -42,3 +42,11 @@ type Y {
 input YInput {
   z: ID
 }
+
+type Mutation {
+  m2(x: ID, y: YInput): M
+}
+
+type M {
+  n: String
+}

apollo-federation/src/sources/connect/expand/mod.rs

@@ -48,7 +48,10 @@ pub enum ExpansionResult {
 /// each connector is separated into its own unique subgraph with relevant GraphQL directives to enforce
 /// field dependencies and response structures. This allows for satisfiability and validation to piggy-back
 /// off of existing functionality in a reproducable way.
-pub fn expand_connectors(supergraph_str: &str) -> Result<ExpansionResult, FederationError> {
+pub fn expand_connectors(
+    supergraph_str: &str,
+    api_schema_options: &ApiSchemaOptions,
+) -> Result<ExpansionResult, FederationError> {
     // TODO: Don't rely on finding the URL manually to short out
     let connect_url = ConnectSpec::identity();
     let connect_url = format!("{}/{}/v", connect_url.domain, connect_url.name);
@@ -57,11 +60,7 @@ pub fn expand_connectors(supergraph_str: &str) -> Result<ExpansionResult, Federa
     }
 
     let supergraph = Supergraph::new(supergraph_str)?;
-    let api_schema = supergraph.to_api_schema(ApiSchemaOptions {
-        // TODO: Get these options from the router?
-        include_defer: true,
-        include_stream: false,
-    })?;
+    let api_schema = supergraph.to_api_schema(api_schema_options.clone())?;
 
     let (connect_subgraphs, graphql_subgraphs): (Vec<_>, Vec<_>) = supergraph
         .extract_subgraphs()?
@@ -314,6 +313,13 @@ mod helpers {
                 .as_ref()
                 .map(|m| m.name.clone())
                 .unwrap_or(name!("Query"));
+            let mutation_alias = self
+                .original_schema
+                .schema()
+                .schema_definition
+                .mutation
+                .as_ref()
+                .map(|m| m.name.clone());
 
             let field = &connector.id.directive.field;
             let field_def = field.get(self.original_schema.schema())?;
@@ -377,17 +383,34 @@ mod helpers {
 
             self.insert_query_for_field(&mut schema, &query_alias, &parent_object, field_def)?;
 
-            let root = SchemaRootDefinitionPosition {
+            let query_root = SchemaRootDefinitionPosition {
                 root_kind: SchemaRootDefinitionKind::Query,
             };
-            root.insert(
+            query_root.insert(
                 &mut schema,
                 ComponentName {
                     origin: ComponentOrigin::Definition,
                     name: query_alias,
                 },
             )?;
 
+            if let Some(mutation_alias) = mutation_alias {
+                // only add the mutation root definition if we've added the
+                // type to this schema
+                if schema.get_type(mutation_alias.clone()).is_ok() {
+                    let mutation_root = SchemaRootDefinitionPosition {
+                        root_kind: SchemaRootDefinitionKind::Mutation,
+                    };
+                    mutation_root.insert(
+                        &mut schema,
+                        ComponentName {
+                            origin: ComponentOrigin::Definition,
+                            name: mutation_alias,
+                        },
+                    )?;
+                }
+            }
+
             // Process any outputs needed by the connector
             self.process_outputs(
                 &mut schema,

apollo-federation/src/sources/connect/expand/tests/mod.rs

@@ -6,6 +6,7 @@ use insta::glob;
 
 use crate::sources::connect::expand::expand_connectors;
 use crate::sources::connect::expand::ExpansionResult;
+use crate::ApiSchemaOptions;
 
 #[test]
 fn it_expand_supergraph() {
@@ -16,7 +17,7 @@ fn it_expand_supergraph() {
                 raw_sdl,
                 api_schema,
                 connectors,
-            } = expand_connectors(&to_expand).unwrap()
+            } = expand_connectors(&to_expand, &ApiSchemaOptions { include_defer: true, ..Default::default() }).unwrap()
             else {
                 panic!("expected expansion to actually expand subgraphs for {path:?}");
             };
@@ -33,7 +34,7 @@ fn it_ignores_supergraph() {
     insta::with_settings!({prepend_module_to_snapshot => false}, {
         glob!("schemas/ignore", "*.graphql", |path| {
             let to_ignore = read_to_string(path).unwrap();
-            let ExpansionResult::Unchanged = expand_connectors(&to_ignore).unwrap() else {
+            let ExpansionResult::Unchanged = expand_connectors(&to_ignore, &ApiSchemaOptions::default()).unwrap() else {
                 panic!("expected expansion to ignore non-connector supergraph for {path:?}");
             };
         });

apollo-federation/src/sources/connect/expand/tests/snapshots/it_expand_supergraph@realistic.graphql-3.snap

@@ -5,6 +5,7 @@ input_file: apollo-federation/src/sources/connect/expand/tests/schemas/expand/re
 ---
 schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) @link(url: "https://specs.apollo.dev/inaccessible/v0.2", for: SECURITY) @join__directive(graphs: [], name: "link", args: {url: "https://specs.apollo.dev/connect/v0.1"}) {
   query: Query
+  mutation: Mutation
 }
 
 directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA

apollo-router/src/plugins/connectors/testdata/mutation.graphql

@@ -25,6 +25,13 @@ directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on
 
 directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
 
+type CreateUserPayload
+  @join__type(graph: CONNECTORS)
+{
+  success: Boolean!
+  user: User!
+}
+
 input join__ContextArgument {
   name: String!
   type: String!
@@ -59,18 +66,20 @@ enum link__Purpose {
 type Mutation
   @join__type(graph: CONNECTORS)
 {
-  createUser(name: String!): User @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "json", http: {POST: "/user", body: "username: $args.name"}, selection: "id\nname: username"})
+  createUser(name: String!): CreateUserPayload! @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "json", http: {POST: "/user", body: "username: $args.name"}, selection: "success: $(true)\nuser: {\n  id\n  name: username\n}"})
 }
 
 type Query
   @join__type(graph: CONNECTORS)
 {
   users: [User] @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "json", http: {GET: "/users"}, selection: "id name"})
+  user(id: ID!): User @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "json", http: {GET: "/users/{$args.id}"}, selection: "id name email", entity: true})
 }
 
 type User
   @join__type(graph: CONNECTORS)
 {
   id: ID!
   name: String
+  email: String
 }

apollo-router/src/plugins/connectors/testdata/mutation.yaml

@@ -4,7 +4,7 @@ subgraphs:
     schema:
       sdl: |
         extend schema
-          @link(url: "https://specs.apollo.dev/federation/v2.7")
+          @link(url: "https://specs.apollo.dev/federation/v2.10")
           @link(
             url: "https://specs.apollo.dev/connect/v0.1"
             import: ["@connect", "@source"]
@@ -19,15 +19,23 @@ subgraphs:
         type User {
           id: ID!
           name: String
+          email: String
         }
 
         type Query {
           users: [User]
             @connect(source: "json", http: { GET: "/users" }, selection: "id name")
+          user(id: ID!): User
+            @connect(
+              source: "json"
+              http: { GET: "/users/{$$args.id}" }
+              selection: "id name email"
+              entity: true
+            )
         }
 
         type Mutation {
-          createUser(name: String!): User
+          createUser(name: String!): CreateUserPayload!
             @connect(
               source: "json"
               http: {
@@ -37,8 +45,16 @@ subgraphs:
                 """
               }
               selection: """
-                id
-                name: username
+                success: $(true)
+                user: {
+                  id
+                  name: username
+                }
               """
             )
         }
+
+        type CreateUserPayload {
+          success: Boolean!
+          user: User!
+        }

apollo-router/src/plugins/connectors/tests/mod.rs

@@ -801,8 +801,11 @@ async fn test_mutation() {
         &mock_server.uri(),
         "mutation CreateUser($name: String!) {
             createUser(name: $name) {
-                id
-                name
+                success
+                user {
+                  id
+                  name
+                }
             }
         }",
         serde_json_bytes::json!({ "name": "New User" })
@@ -818,8 +821,11 @@ async fn test_mutation() {
     {
       "data": {
         "createUser": {
-          "id": 3,
-          "name": "New User"
+          "success": true,
+          "user": {
+            "id": 3,
+            "name": "New User"
+          }
         }
       }
     }

apollo-router/src/spec/schema.rs

@@ -54,7 +54,13 @@ impl Schema {
     ) -> Result<Self, SchemaError> {
         let start = Instant::now();
 
-        let expansion = expand_connectors(&raw_sdl.sdl).map_err(SchemaError::Connector)?;
+        let api_schema_options = ApiSchemaOptions {
+            include_defer: config.supergraph.defer_support,
+            ..Default::default()
+        };
+
+        let expansion =
+            expand_connectors(&raw_sdl.sdl, &api_schema_options).map_err(SchemaError::Connector)?;
         let preserved_launch_id = raw_sdl.launch_id.clone();
         let (raw_sdl, api_schema, connectors) = match expansion {
             ExpansionResult::Expanded {
@@ -149,16 +155,11 @@ impl Schema {
         let schema_id = Arc::new(Schema::schema_id(&raw_sdl.sdl));
 
         let api_schema = api_schema.map(Ok).unwrap_or_else(|| {
-            supergraph
-                .to_api_schema(ApiSchemaOptions {
-                    include_defer: config.supergraph.defer_support,
-                    ..Default::default()
-                })
-                .map_err(|e| {
-                    SchemaError::Api(format!(
-                        "The supergraph schema failed to produce a valid API schema: {e}"
-                    ))
-                })
+            supergraph.to_api_schema(api_schema_options).map_err(|e| {
+                SchemaError::Api(format!(
+                    "The supergraph schema failed to produce a valid API schema: {e}"
+                ))
+            })
         })?;
 
         Ok(Schema {

apollo-router/tests/samples/enterprise/connectors-defer/README.md

@@ -0,0 +1 @@
+This tests using defer and connectors. It uses a mutation because there was an expansion bug with mutation root type definitions that appeared only when using defer.

apollo-router/tests/samples/enterprise/connectors-defer/configuration.yaml

@@ -0,0 +1,15 @@
+include_subgraph_errors:
+  all: true
+
+preview_connectors:
+  subgraphs:
+    connectors:
+      sources:
+        jsonPlaceholder:
+          override_url: http://localhost:4007
+
+telemetry:
+  exporters:
+    logging:
+      stdout:
+        format: text

apollo-router/tests/samples/enterprise/connectors-defer/http_snapshots.json

@@ -0,0 +1,38 @@
+[
+  {
+    "request": {
+      "method": "POST",
+      "path": "/",
+      "body": null
+    },
+    "response": {
+      "status": 200,
+      "headers": {
+        "content-type": ["application/json; charset=utf-8"]
+      },
+      "body": {
+        "f": "1",
+        "entity": {
+          "id": "2"
+        }
+      }
+    }
+  },
+  {
+    "request": {
+      "method": "GET",
+      "path": "/e/2",
+      "body": null
+    },
+    "response": {
+      "status": 200,
+      "headers": {
+        "content-type": ["application/json; charset=utf-8"]
+      },
+      "body": {
+        "id": "2",
+        "f": "3"
+      }
+    }
+  }
+]