fix: update connector spec to allow re-entry

composition-js/src/__tests__/connectors.test.ts

@@ -1,123 +1,155 @@
 import { composeServices } from "../compose";
-import { printSchema } from "@apollo/federation-internals";
+import { buildSubgraph, printSchema } from "@apollo/federation-internals";
 import { parse } from "graphql/index";
 
 describe("connect spec and join__directive", () => {
-  it("composes", () => {
-    const subgraphs = [
-      {
-        name: "with-connectors",
-        typeDefs: parse(`
-                    extend schema
-                    @link(
-                        url: "https://specs.apollo.dev/federation/v2.10"
-                        import: ["@key"]
-                    )
-                    @link(
-                        url: "https://specs.apollo.dev/connect/v0.1"
-                        import: ["@connect", "@source"]
-                    )
-                    @source(name: "v1", http: { baseURL: "http://v1" })
+  const subgraphSdl = `
+      extend schema
+      @link(
+          url: "https://specs.apollo.dev/federation/v2.10"
+          import: ["@key"]
+      )
+      @link(
+          url: "https://specs.apollo.dev/connect/v0.1"
+          import: ["@connect", "@source"]
+      )
+      @source(name: "v1", http: { baseURL: "http://v1" })
+
+      type Query {
+          resources: [Resource!]!
+          @connect(source: "v1", http: { GET: "/resources" }, selection: "")
+      }
 
-                    type Query {
-                        resources: [Resource!]!
-                        @connect(source: "v1", http: { GET: "/resources" }, selection: "")
-                    }
+      type Resource @key(fields: "id") {
+          id: ID!
+          name: String!
+      }
+  `;
+
+  const expectedSupergraphSdl = `
+    "schema
+      @link(url: \\"https://specs.apollo.dev/link/v1.0\\")
+      @link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
+      @link(url: \\"https://specs.apollo.dev/connect/v0.2\\", for: EXECUTION)
+      @join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]})
+      @join__directive(graphs: [WITH_CONNECTORS], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
+    {
+      query: Query
+    }
 
-                    type Resource @key(fields: "id") {
-                        id: ID!
-                        name: String!
-                    }
-                `),
-      },
-    ];
+    directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
 
-    const result = composeServices(subgraphs);
-    expect(result.errors ?? []).toEqual([]);
-    const printed = printSchema(result.schema!);
-    expect(printed).toMatchInlineSnapshot(`
-      "schema
-        @link(url: \\"https://specs.apollo.dev/link/v1.0\\")
-        @link(url: \\"https://specs.apollo.dev/join/v0.5\\", for: EXECUTION)
-        @link(url: \\"https://specs.apollo.dev/connect/v0.2\\", for: EXECUTION)
-        @join__directive(graphs: [WITH_CONNECTORS], name: \\"link\\", args: {url: \\"https://specs.apollo.dev/connect/v0.1\\", import: [\\"@connect\\", \\"@source\\"]})
-        @join__directive(graphs: [WITH_CONNECTORS], name: \\"source\\", args: {name: \\"v1\\", http: {baseURL: \\"http://v1\\"}})
-      {
-        query: Query
-      }
+    directive @join__graph(name: String!, url: String!) on ENUM_VALUE
 
-      directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
+    directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
 
-      directive @join__graph(name: String!, url: String!) on ENUM_VALUE
+    directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String, contextArguments: [join__ContextArgument!]) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
 
-      directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
+    directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE
 
-      directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String, contextArguments: [join__ContextArgument!]) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
+    directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION
 
-      directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE
+    directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE
 
-      directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION
+    directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION
 
-      directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE
+    enum link__Purpose {
+      \\"\\"\\"
+      \`SECURITY\` features provide metadata necessary to securely resolve fields.
+      \\"\\"\\"
+      SECURITY
 
-      directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION
+      \\"\\"\\"
+      \`EXECUTION\` features provide metadata necessary for operation execution.
+      \\"\\"\\"
+      EXECUTION
+    }
 
-      enum link__Purpose {
-        \\"\\"\\"
-        \`SECURITY\` features provide metadata necessary to securely resolve fields.
-        \\"\\"\\"
-        SECURITY
+    scalar link__Import
 
-        \\"\\"\\"
-        \`EXECUTION\` features provide metadata necessary for operation execution.
-        \\"\\"\\"
-        EXECUTION
-      }
+    enum join__Graph {
+      WITH_CONNECTORS @join__graph(name: \\"with-connectors\\", url: \\"\\")
+    }
 
-      scalar link__Import
+    scalar join__FieldSet
 
-      enum join__Graph {
-        WITH_CONNECTORS @join__graph(name: \\"with-connectors\\", url: \\"\\")
-      }
+    scalar join__DirectiveArguments
 
-      scalar join__FieldSet
+    scalar join__FieldValue
 
-      scalar join__DirectiveArguments
+    input join__ContextArgument {
+      name: String!
+      type: String!
+      context: String!
+      selection: join__FieldValue!
+    }
 
-      scalar join__FieldValue
+    type Query
+      @join__type(graph: WITH_CONNECTORS)
+    {
+      resources: [Resource!]! @join__directive(graphs: [WITH_CONNECTORS], name: \\"connect\\", args: {source: \\"v1\\", http: {GET: \\"/resources\\"}, selection: \\"\\"})
+    }
 
-      input join__ContextArgument {
-        name: String!
-        type: String!
-        context: String!
-        selection: join__FieldValue!
-      }
+    type Resource
+      @join__type(graph: WITH_CONNECTORS, key: \\"id\\")
+    {
+      id: ID!
+      name: String!
+    }"
+  `;
+
+  const expectedApiSdl = `
+    "type Query {
+      resources: [Resource!]!
+    }
 
-      type Query
-        @join__type(graph: WITH_CONNECTORS)
-      {
-        resources: [Resource!]! @join__directive(graphs: [WITH_CONNECTORS], name: \\"connect\\", args: {source: \\"v1\\", http: {GET: \\"/resources\\"}, selection: \\"\\"})
-      }
+    type Resource {
+      id: ID!
+      name: String!
+    }"
+  `;
 
-      type Resource
-        @join__type(graph: WITH_CONNECTORS, key: \\"id\\")
+  it("composes", () => {
+    const subgraphs = [
       {
-        id: ID!
-        name: String!
-      }"
-    `);
+        name: "with-connectors",
+        typeDefs: parse(subgraphSdl),
+      },
+    ];
+
+    const result = composeServices(subgraphs);
+    expect(result.errors ?? []).toEqual([]);
+    const printed = printSchema(result.schema!);
+    expect(printed).toMatchInlineSnapshot(expectedSupergraphSdl);
 
     if (result.schema) {
-      expect(printSchema(result.schema.toAPISchema())).toMatchInlineSnapshot(`
-                      "type Query {
-                        resources: [Resource!]!
-                      }
+      expect(printSchema(result.schema.toAPISchema()))
+        .toMatchInlineSnapshot(expectedApiSdl);
+    }    
+  });
 
-                      type Resource {
-                        id: ID!
-                        name: String!
-                      }"
-                  `);
+  it("composes with completed definitions", () => {
+    const completedSubgraphSdl = printSchema(buildSubgraph(
+      "with-connectors",
+      "",
+      subgraphSdl,
+    ).schema);
+
+    const subgraphs = [
+      {
+        name: "with-connectors",
+        typeDefs: parse(completedSubgraphSdl),
+      },
+    ];
+
+    const result = composeServices(subgraphs);
+    expect(result.errors ?? []).toEqual([]);
+    const printed = printSchema(result.schema!);
+    expect(printed).toMatchInlineSnapshot(expectedSupergraphSdl);
+
+    if (result.schema) {
+      expect(printSchema(result.schema.toAPISchema()))
+        .toMatchInlineSnapshot(expectedApiSdl);
     }
   });
 

internals-js/src/buildSchema.ts

@@ -56,6 +56,9 @@ import {
 } from "./definitions";
 import { ERRORS, errorCauses, withModifiedErrorNodes } from "./error";
 import { introspectionTypeNames } from "./introspection";
+import { coreFeatureDefinitionIfKnown } from "./knownCoreFeatures";
+import { connectIdentity } from "./specs/connectSpec";
+
 
 function buildValue(value?: ValueNode): any {
   return value ? valueFromASTUntyped(value) : undefined;
@@ -143,6 +146,48 @@ export function buildSchemaFromAST(
     buildSchemaDefinitionInner(schemaExtension, schema.schemaDefinition, errors, schema.schemaDefinition.newExtension());
   }
 
+  // The following block of code is a one-off to support input objects in the
+  // connect spec. It will be non-maintainable/bug-prone to do this again, and
+  // has various limitations/unsupported edge cases already.
+  //
+  // There's work to be done to support input objects more generally; please see
+  // https://github.com/apollographql/federation/pull/3311 for more information.
+  const connectFeature = schema.coreFeatures?.getByIdentity(connectIdentity);
+  const handledConnectTypeNames = new Set<string>();
+  if (connectFeature) {
+    const connectFeatureDefinition =
+      coreFeatureDefinitionIfKnown(connectFeature.url);
+    if (connectFeatureDefinition) {
+      const connectTypeNamesInSchema = new Set(
+        connectFeatureDefinition.typeSpecs()
+          .map(({ name }) => connectFeature.typeNameInSchema(name))
+      );
+      for (const typeNode of typeDefinitions) {
+        if (connectTypeNamesInSchema.has(typeNode.name.value)
+          && typeNode.kind === 'InputObjectTypeDefinition'
+        ) {
+          handledConnectTypeNames.add(typeNode.name.value)
+        } else {
+          continue;
+        }
+        buildNamedTypeInner(typeNode, schema.type(typeNode.name.value)!, schema.blueprint, errors);
+      }
+      for (const typeExtensionNode of typeExtensions) {
+        if (connectTypeNamesInSchema.has(typeExtensionNode.name.value)
+          && typeExtensionNode.kind === 'InputObjectTypeExtension'
+        ) {
+          handledConnectTypeNames.add(typeExtensionNode.name.value)
+        } else {
+          continue;
+        }
+        const toExtend = schema.type(typeExtensionNode.name.value)!;
+        const extension = toExtend.newExtension();
+        extension.sourceAST = typeExtensionNode;
+        buildNamedTypeInner(typeExtensionNode, toExtend, schema.blueprint, errors, extension);
+      }
+    }
+  }
+
   // The following is a no-op for "standard" schema, but for federation subgraphs, this is where we handle the auto-addition
   // of imported federation directive definitions. That is why we have avoid looking at directive applications within
   // directive definition earlier: if one of those application was of an imported federation directive, the definition
@@ -155,9 +200,15 @@ export function buildSchemaFromAST(
   }
 
   for (const typeNode of typeDefinitions) {
+    if (handledConnectTypeNames.has(typeNode.name.value)) {
+      continue;
+    }
     buildNamedTypeInner(typeNode, schema.type(typeNode.name.value)!, schema.blueprint, errors);
   }
   for (const typeExtensionNode of typeExtensions) {
+    if (handledConnectTypeNames.has(typeExtensionNode.name.value)) {
+      continue;
+    }
     const toExtend = schema.type(typeExtensionNode.name.value)!;
     const extension = toExtend.newExtension();
     extension.sourceAST = typeExtensionNode;

internals-js/src/directiveAndTypeSpecification.ts

@@ -66,7 +66,13 @@ export type FieldSpecification = {
   args?: ResolvedArgumentSpecification[],
 }
 
-type ResolvedArgumentSpecification = {
+export type ResolvedArgumentSpecification = {
+  name: string,
+  type: InputType,
+  defaultValue?: any,
+}
+
+export type InputFieldSpecification = {
   name: string,
   type: InputType,
   defaultValue?: any,
@@ -338,7 +344,7 @@ export function createEnumTypeSpecification({
   }
 }
 
-function ensureSameTypeKind(expected: NamedType['kind'], actual: NamedType): GraphQLError[] {
+export function ensureSameTypeKind(expected: NamedType['kind'], actual: NamedType): GraphQLError[] {
   return expected === actual.kind
     ? []
     : [