From b6ccf9884e5543f6a044ad830c9440de65c7430a Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Wed, 1 Oct 2025 11:53:05 -0500 Subject: [PATCH 1/3] fix: update connector spec to allow re-entry Updates connector spec to follow the same patterns as other federation spec blueprints (i.e. register types/directives in the constructor and use default logic for adding them to the schema that checks whether they need to be added or not). --- .../src/directiveAndTypeSpecification.ts | 68 ++- internals-js/src/specs/connectSpec.ts | 396 ++++++++++++------ 2 files changed, 338 insertions(+), 126 deletions(-) diff --git a/internals-js/src/directiveAndTypeSpecification.ts b/internals-js/src/directiveAndTypeSpecification.ts index 57b728dcc..586cfa277 100644 --- a/internals-js/src/directiveAndTypeSpecification.ts +++ b/internals-js/src/directiveAndTypeSpecification.ts @@ -3,10 +3,10 @@ import { ArgumentDefinition, CoreFeature, DirectiveDefinition, - EnumType, + EnumType, InputObjectType, InputType, isCustomScalarType, - isEnumType, + isEnumType, isInputObjectType, isListType, isNonNullType, isObjectType, @@ -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, @@ -251,6 +257,62 @@ export function createObjectTypeSpecification({ } } +export function createInputObjectTypeSpecification({ + name, + inputFieldsFct, +}: { + name: string, + inputFieldsFct: (schema: Schema, feature?: CoreFeature) => InputFieldSpecification[], +}): TypeSpecification { + return { + name, + checkOrAdd: (schema: Schema, feature?: CoreFeature, asBuiltIn?: boolean) => { + const actualName = feature?.typeNameInSchema(name) ?? name; + const expectedFields = inputFieldsFct(schema, feature); + const existing = schema.type(actualName); + if (existing) { + let errors = ensureSameTypeKind('InputObjectType', existing); + if (errors.length > 0) { + return errors; + } + assert(isInputObjectType(existing), 'Should be an input object type'); + for (const { name: field_name, type, defaultValue } of expectedFields) { + const existingField = existing.field(field_name); + if (!existingField) { + errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition of type ${name}: missing input field ${field_name}`, + { nodes: existing.sourceAST }, + )); + continue; + } + if (!sameType(type, existingField.type!)) { + errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for field ${field_name} of type ${name}: should have type ${type} but found type ${existingField.type}`, + { nodes: existingField.sourceAST }, + )); + } + if (defaultValue !== existingField.defaultValue) { + errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for field ${field_name} of type ${name}: should have default value ${defaultValue} but found type ${existingField.defaultValue}`, + { nodes: existingField.sourceAST }, + )); + } + } + return errors; + } else { + const createdType = schema.addType(new InputObjectType(actualName, asBuiltIn)); + for (const { name, type, defaultValue } of expectedFields) { + const newField = createdType.addField(name, type); + if (defaultValue) { + newField.defaultValue = defaultValue; + } + } + return []; + } + }, + } +} + export function createUnionTypeSpecification({ name, membersFct, diff --git a/internals-js/src/specs/connectSpec.ts b/internals-js/src/specs/connectSpec.ts index 43111d1ab..6d686d81a 100644 --- a/internals-js/src/specs/connectSpec.ts +++ b/internals-js/src/specs/connectSpec.ts @@ -1,4 +1,4 @@ -import { DirectiveLocation, GraphQLError } from 'graphql'; +import { DirectiveLocation } from 'graphql'; import { CorePurpose, FeatureDefinition, @@ -7,17 +7,16 @@ import { FeatureVersion, } from './coreSpec'; import { - Schema, - NonNullType, InputObjectType, - InputFieldDefinition, ListType, + NonNullType, ScalarType, } from '../definitions'; import { registerKnownFeature } from '../knownCoreFeatures'; import { - createDirectiveSpecification, + createDirectiveSpecification, createInputObjectTypeSpecification, createScalarTypeSpecification, } from '../directiveAndTypeSpecification'; +import {assert} from "../utils"; export const connectIdentity = 'https://specs.apollo.dev/connect'; @@ -41,60 +40,40 @@ export class ConnectSpecDefinition extends FeatureDefinition { minimumFederationVersion, ); - this.registerDirective( - createDirectiveSpecification({ - name: CONNECT, - locations: [DirectiveLocation.FIELD_DEFINITION], - repeatable: true, - // We "compose" these directives using the `@join__directive` mechanism, - // so they do not need to be composed in the way passing `composes: true` - // here implies. - composes: false, - }), - ); - - this.registerDirective( - createDirectiveSpecification({ - name: SOURCE, - locations: [DirectiveLocation.SCHEMA], - repeatable: true, - composes: false, - }), - ); - + /* scalar URLPathTemplate */ this.registerType( - createScalarTypeSpecification({ name: URL_PATH_TEMPLATE }), + createScalarTypeSpecification({ name: URL_PATH_TEMPLATE }), ); - this.registerType(createScalarTypeSpecification({ name: JSON_SELECTION })); - this.registerType({ name: CONNECT_HTTP, checkOrAdd: () => [] }); - this.registerType({ name: SOURCE_HTTP, checkOrAdd: () => [] }); - this.registerType({ name: HTTP_HEADER_MAPPING, checkOrAdd: () => [] }); - } - - addElementsToSchema(schema: Schema): GraphQLError[] { - /* scalar URLPathTemplate */ - const URLPathTemplate = this.addScalarType(schema, URL_PATH_TEMPLATE); - /* scalar JSONSelection */ - const JSONSelection = this.addScalarType(schema, JSON_SELECTION); + this.registerType(createScalarTypeSpecification({ name: JSON_SELECTION })); /* - directive @connect( - source: String - http: ConnectHTTP - selection: JSONSelection! - entity: Boolean = false - errors: ConnectorErrors - ) repeatable on FIELD_DEFINITION - | OBJECT # added in v0.2, validation enforced in rust + input ConnectorErrors { + message: JSONSelection + extensions: JSONSelection + } */ - const connect = this.addDirective(schema, CONNECT).addLocations( - DirectiveLocation.FIELD_DEFINITION, - DirectiveLocation.OBJECT, + this.registerType( + createInputObjectTypeSpecification({ + name: CONNECTOR_ERRORS, + inputFieldsFct: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @connect spec"); + const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); + const jsonSelectionType = schema.typeOfKind(jsonSelectionName, 'ScalarType'); + assert(jsonSelectionType, () => `Expected "${jsonSelectionName}" to be defined`); + return [ + { + name: 'message', + type: jsonSelectionType + }, + { + name: 'extensions', + type: jsonSelectionType + }, + ] + } + }) ); - connect.repeatable = true; - - connect.addArgument(SOURCE, schema.stringType()); /* input HTTPHeaderMapping { @@ -103,15 +82,86 @@ export class ConnectSpecDefinition extends FeatureDefinition { value: String } */ - const HTTPHeaderMapping = schema.addType( - new InputObjectType(this.typeNameInSchema(schema, HTTP_HEADER_MAPPING)!), + this.registerType( + createInputObjectTypeSpecification({ + name: HTTP_HEADER_MAPPING, + inputFieldsFct: (schema) => [ + { + name: 'name', + type: new NonNullType(schema.stringType()) + }, + { + name: 'from', + type: schema.stringType() + }, + { + name: 'value', + type: schema.stringType() + }, + ] + }) + ); + + /* + input ConnectBatch { + maxSize: Int + } + */ + this.registerType( + createInputObjectTypeSpecification({ + name: CONNECT_BATCH, + inputFieldsFct: (schema) => [ + { + name: 'maxSize', + type: schema.intType() + } + ] + }) + ) + + /* + input SourceHTTP { + baseURL: String! + headers: [HTTPHeaderMapping!] + + # added in v0.2 + path: JSONSelection + query: JSONSelection + } + */ + this.registerType( + createInputObjectTypeSpecification({ + name: SOURCE_HTTP, + inputFieldsFct: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @link spec"); + const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); + const jsonSelectionType = schema.typeOfKind(jsonSelectionName, 'ScalarType'); + assert(jsonSelectionType, () => `Expected "${jsonSelectionType}" to be defined`); + + const httpHeaderMappingName = feature.typeNameInSchema(HTTP_HEADER_MAPPING); + const httpHeaderMappingType = schema.typeOfKind(httpHeaderMappingName, 'InputObjectType'); + assert(httpHeaderMappingType, () => `Expected "${httpHeaderMappingType}" to be defined`); + return [ + { + name: 'baseURL', + type: schema.stringType() + }, + { + name: 'headers', + type: new ListType(new NonNullType(httpHeaderMappingType)) + }, + { + name: 'path', + type: jsonSelectionType + }, + { + name: 'queryParams', + type: jsonSelectionType + } + ]; + } + }) ); - HTTPHeaderMapping.addField(new InputFieldDefinition('name')).type = - new NonNullType(schema.stringType()); - HTTPHeaderMapping.addField(new InputFieldDefinition('from')).type = - schema.stringType(); - HTTPHeaderMapping.addField(new InputFieldDefinition('value')).type = - schema.stringType(); /* input ConnectHTTP { @@ -128,79 +178,179 @@ export class ConnectSpecDefinition extends FeatureDefinition { query: JSONSelection } */ - const ConnectHTTP = schema.addType( - new InputObjectType(this.typeNameInSchema(schema, CONNECT_HTTP)!), + this.registerType( + createInputObjectTypeSpecification({ + name: CONNECT_HTTP, + inputFieldsFct: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @link spec"); + const urlPathTemplateName = feature.typeNameInSchema(URL_PATH_TEMPLATE); + const urlPathTemplateType = schema.typeOfKind(urlPathTemplateName, 'ScalarType'); + assert(urlPathTemplateType, () => `Expected "${urlPathTemplateType}" to be defined`); + + const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); + const jsonSelectionType = schema.typeOfKind(jsonSelectionName, 'ScalarType'); + assert(jsonSelectionType, () => `Expected "${jsonSelectionType}" to be defined`); + + const httpHeaderMappingName = feature.typeNameInSchema(HTTP_HEADER_MAPPING); + const httpHeaderMappingType = schema.typeOfKind(httpHeaderMappingName, 'InputObjectType'); + assert(httpHeaderMappingType, () => `Expected "${httpHeaderMappingType}" to be defined`); + return [ + { + name: 'GET', + type: urlPathTemplateType + }, + { + name: 'POST', + type: urlPathTemplateType + }, + { + name: 'PUT', + type: urlPathTemplateType + }, + { + name: 'PATCH', + type: urlPathTemplateType + }, + { + name: 'DELETE', + type: urlPathTemplateType + }, + { + name: 'body', + type: jsonSelectionType + }, + { + name: 'headers', + type: new ListType(new NonNullType(httpHeaderMappingType)) + }, + { + name: 'path', + type: jsonSelectionType + }, + { + name: 'queryParams', + type: jsonSelectionType + }, + ]; + } + }) ); - ConnectHTTP.addField(new InputFieldDefinition('GET')).type = - URLPathTemplate; - ConnectHTTP.addField(new InputFieldDefinition('POST')).type = - URLPathTemplate; - ConnectHTTP.addField(new InputFieldDefinition('PUT')).type = - URLPathTemplate; - ConnectHTTP.addField(new InputFieldDefinition('PATCH')).type = - URLPathTemplate; - ConnectHTTP.addField(new InputFieldDefinition('DELETE')).type = - URLPathTemplate; - ConnectHTTP.addField(new InputFieldDefinition('body')).type = JSONSelection; - ConnectHTTP.addField(new InputFieldDefinition('headers')).type = - new ListType(new NonNullType(HTTPHeaderMapping)); - - ConnectHTTP.addField(new InputFieldDefinition('path')).type = JSONSelection; - ConnectHTTP.addField(new InputFieldDefinition('queryParams')).type = - JSONSelection; - - connect.addArgument('http', new NonNullType(ConnectHTTP)); - - const ConnectBatch = schema.addType(new InputObjectType(this.typeNameInSchema(schema, CONNECT_BATCH)!)); - ConnectBatch.addField(new InputFieldDefinition('maxSize')).type = schema.intType(); - connect.addArgument('batch', ConnectBatch); - - const ConnectorErrors = schema.addType(new InputObjectType(this.typeNameInSchema(schema, CONNECTOR_ERRORS)!)); - ConnectorErrors.addField(new InputFieldDefinition('message')).type = JSONSelection; - ConnectorErrors.addField(new InputFieldDefinition('extensions')).type = JSONSelection; - connect.addArgument('errors', ConnectorErrors); - - connect.addArgument('selection', new NonNullType(JSONSelection)); - connect.addArgument('entity', schema.booleanType(), false); /* - directive @source( - name: String! - http: ConnectHTTP + directive @connect( + source: String + http: ConnectHTTP! + batch: ConnectBatch + selection: JSONSelection! + entity: Boolean = false errors: ConnectorErrors - ) repeatable on SCHEMA + ) repeatable on FIELD_DEFINITION + | OBJECT # added in v0.2, validation enforced in rust */ - const source = this.addDirective(schema, SOURCE).addLocations( - DirectiveLocation.SCHEMA, + this.registerDirective( + createDirectiveSpecification({ + name: CONNECT, + locations: [DirectiveLocation.FIELD_DEFINITION, DirectiveLocation.OBJECT], + repeatable: true, + args: [ + { + name: 'source', + type: (schema) => schema.stringType() + }, + { + name: 'http', + type: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @connect spec"); + const connectHttpName = feature.typeNameInSchema(CONNECT_HTTP); + const connectHttpType = schema.typeOfKind(connectHttpName, "InputObjectType"); + assert(connectHttpType, () => `Expected "${connectHttpName}" to be defined`); + return new NonNullType(connectHttpType); + } + }, + { + name: 'batch', + type: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @connect spec"); + const connectBatchName = feature.typeNameInSchema(CONNECT_BATCH); + const connectBatchType = schema.typeOfKind(connectBatchName, "InputObjectType"); + assert(connectBatchType, () => `Expected "${connectBatchName}" to be defined`); + return connectBatchType; + } + }, + { + name: 'selection', + type: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @connect spec"); + const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); + const jsonSelectionType = schema.typeOfKind(jsonSelectionName, "ScalarType"); + assert(jsonSelectionType, () => `Expected "${jsonSelectionName}" to be defined`); + return jsonSelectionType; + } + }, + { + name: 'entity', + type: (schema) => schema.booleanType(), + defaultValue: false + }, + { + name: 'errors', + type: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @connect spec"); + const connectorErrorsName = feature.typeNameInSchema(CONNECTOR_ERRORS); + const connectorErrorsType = schema.typeOfKind(connectorErrorsName, "InputObjectType"); + assert(connectorErrorsType, () => `Expected "${connectorErrorsName}" to be defined`); + return connectorErrorsType; + } + } + ], + // We "compose" these directives using the `@join__directive` mechanism, + // so they do not need to be composed in the way passing `composes: true` + // here implies. + composes: false, + }), ); - source.repeatable = true; - source.addArgument('name', new NonNullType(schema.stringType())); /* - input SourceHTTP { - baseURL: String! - headers: [HTTPHeaderMapping!] - - # added in v0.2 - path: JSONSelection - query: JSONSelection - } + directive @source( + name: String! + http: SourceHttp! + errors: ConnectorErrors + ) repeatable on SCHEMA */ - const SourceHTTP = schema.addType( - new InputObjectType(this.typeNameInSchema(schema, SOURCE_HTTP)!), + this.registerDirective( + createDirectiveSpecification({ + name: SOURCE, + locations: [DirectiveLocation.SCHEMA], + repeatable: true, + composes: false, + args: [ + { + name: 'name', + type: (schema) => new NonNullType(schema.stringType()) + }, + { + name: 'http', + type: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @connect spec"); + const sourceHttpName = feature.typeNameInSchema(SOURCE_HTTP); + const sourceHttpType = schema.typeOfKind(sourceHttpName, "InputObjectType"); + assert(sourceHttpType, () => `Expected "${sourceHttpName}" to be defined`); + return new NonNullType(sourceHttpType); + } + }, + { + name: 'errors', + type: (schema, feature) => { + assert(feature, "Shouldn't be added without being attached to a @connect spec"); + const connectorErrorsName = feature.typeNameInSchema(CONNECTOR_ERRORS); + const connectorErrorsType = schema.typeOfKind(connectorErrorsName, "InputObjectType"); + assert(connectorErrorsType, () => `Expected "${connectorErrorsName}" to be defined`); + return connectorErrorsType; + } + } + ] + }), ); - SourceHTTP.addField(new InputFieldDefinition('baseURL')).type = - new NonNullType(schema.stringType()); - SourceHTTP.addField(new InputFieldDefinition('headers')).type = - new ListType(new NonNullType(HTTPHeaderMapping)); - - SourceHTTP.addField(new InputFieldDefinition('path')).type = JSONSelection; - SourceHTTP.addField(new InputFieldDefinition('queryParams')).type = JSONSelection; - - source.addArgument('http', new NonNullType(SourceHTTP)); - source.addArgument('errors', ConnectorErrors); - - return []; } get defaultCorePurpose(): CorePurpose { From e850c925b17239b91e700288ce00c750d6c2b85b Mon Sep 17 00:00:00 2001 From: dariuszkuc <9501705+dariuszkuc@users.noreply.github.com> Date: Wed, 1 Oct 2025 14:57:16 -0500 Subject: [PATCH 2/3] extract common logic to a helper function --- internals-js/src/specs/connectSpec.ts | 97 ++++++++++----------------- 1 file changed, 35 insertions(+), 62 deletions(-) diff --git a/internals-js/src/specs/connectSpec.ts b/internals-js/src/specs/connectSpec.ts index 6d686d81a..627f59220 100644 --- a/internals-js/src/specs/connectSpec.ts +++ b/internals-js/src/specs/connectSpec.ts @@ -7,9 +7,10 @@ import { FeatureVersion, } from './coreSpec'; import { + CoreFeature, InputObjectType, - ListType, - NonNullType, ScalarType, + ListType, NamedType, + NonNullType, ScalarType, Schema, } from '../definitions'; import { registerKnownFeature } from '../knownCoreFeatures'; import { @@ -40,6 +41,14 @@ export class ConnectSpecDefinition extends FeatureDefinition { minimumFederationVersion, ); + function lookupFeatureTypeInSchema(name: string, kind: T['kind'], schema: Schema, feature?: CoreFeature): T { + assert(feature, `Shouldn't be added without being attached to a @connect spec`); + const typeName = feature.typeNameInSchema(name); + const type = schema.typeOfKind(typeName, kind); + assert(type, () => `Expected "${typeName}" to be defined`); + return type; + } + /* scalar URLPathTemplate */ this.registerType( createScalarTypeSpecification({ name: URL_PATH_TEMPLATE }), @@ -57,10 +66,8 @@ export class ConnectSpecDefinition extends FeatureDefinition { createInputObjectTypeSpecification({ name: CONNECTOR_ERRORS, inputFieldsFct: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @connect spec"); - const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); - const jsonSelectionType = schema.typeOfKind(jsonSelectionName, 'ScalarType'); - assert(jsonSelectionType, () => `Expected "${jsonSelectionName}" to be defined`); + const jsonSelectionType = + lookupFeatureTypeInSchema(JSON_SELECTION, 'ScalarType', schema, feature); return [ { name: 'message', @@ -133,14 +140,10 @@ export class ConnectSpecDefinition extends FeatureDefinition { createInputObjectTypeSpecification({ name: SOURCE_HTTP, inputFieldsFct: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @link spec"); - const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); - const jsonSelectionType = schema.typeOfKind(jsonSelectionName, 'ScalarType'); - assert(jsonSelectionType, () => `Expected "${jsonSelectionType}" to be defined`); - - const httpHeaderMappingName = feature.typeNameInSchema(HTTP_HEADER_MAPPING); - const httpHeaderMappingType = schema.typeOfKind(httpHeaderMappingName, 'InputObjectType'); - assert(httpHeaderMappingType, () => `Expected "${httpHeaderMappingType}" to be defined`); + const jsonSelectionType = + lookupFeatureTypeInSchema(JSON_SELECTION, 'ScalarType', schema, feature); + const httpHeaderMappingType = + lookupFeatureTypeInSchema(HTTP_HEADER_MAPPING, 'InputObjectType', schema, feature); return [ { name: 'baseURL', @@ -182,18 +185,12 @@ export class ConnectSpecDefinition extends FeatureDefinition { createInputObjectTypeSpecification({ name: CONNECT_HTTP, inputFieldsFct: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @link spec"); - const urlPathTemplateName = feature.typeNameInSchema(URL_PATH_TEMPLATE); - const urlPathTemplateType = schema.typeOfKind(urlPathTemplateName, 'ScalarType'); - assert(urlPathTemplateType, () => `Expected "${urlPathTemplateType}" to be defined`); - - const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); - const jsonSelectionType = schema.typeOfKind(jsonSelectionName, 'ScalarType'); - assert(jsonSelectionType, () => `Expected "${jsonSelectionType}" to be defined`); - - const httpHeaderMappingName = feature.typeNameInSchema(HTTP_HEADER_MAPPING); - const httpHeaderMappingType = schema.typeOfKind(httpHeaderMappingName, 'InputObjectType'); - assert(httpHeaderMappingType, () => `Expected "${httpHeaderMappingType}" to be defined`); + const urlPathTemplateType = + lookupFeatureTypeInSchema(URL_PATH_TEMPLATE, 'ScalarType', schema, feature); + const jsonSelectionType = + lookupFeatureTypeInSchema(JSON_SELECTION, 'ScalarType', schema, feature); + const httpHeaderMappingType = + lookupFeatureTypeInSchema(HTTP_HEADER_MAPPING, 'InputObjectType', schema, feature); return [ { name: 'GET', @@ -260,32 +257,20 @@ export class ConnectSpecDefinition extends FeatureDefinition { { name: 'http', type: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @connect spec"); - const connectHttpName = feature.typeNameInSchema(CONNECT_HTTP); - const connectHttpType = schema.typeOfKind(connectHttpName, "InputObjectType"); - assert(connectHttpType, () => `Expected "${connectHttpName}" to be defined`); + const connectHttpType = + lookupFeatureTypeInSchema(CONNECT_HTTP, 'InputObjectType', schema, feature); return new NonNullType(connectHttpType); } }, { name: 'batch', - type: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @connect spec"); - const connectBatchName = feature.typeNameInSchema(CONNECT_BATCH); - const connectBatchType = schema.typeOfKind(connectBatchName, "InputObjectType"); - assert(connectBatchType, () => `Expected "${connectBatchName}" to be defined`); - return connectBatchType; - } + type: (schema, feature) => + lookupFeatureTypeInSchema(CONNECT_BATCH, 'InputObjectType', schema, feature) }, { name: 'selection', - type: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @connect spec"); - const jsonSelectionName = feature.typeNameInSchema(JSON_SELECTION); - const jsonSelectionType = schema.typeOfKind(jsonSelectionName, "ScalarType"); - assert(jsonSelectionType, () => `Expected "${jsonSelectionName}" to be defined`); - return jsonSelectionType; - } + type: (schema, feature) => + lookupFeatureTypeInSchema(JSON_SELECTION, 'ScalarType', schema, feature) }, { name: 'entity', @@ -294,13 +279,8 @@ export class ConnectSpecDefinition extends FeatureDefinition { }, { name: 'errors', - type: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @connect spec"); - const connectorErrorsName = feature.typeNameInSchema(CONNECTOR_ERRORS); - const connectorErrorsType = schema.typeOfKind(connectorErrorsName, "InputObjectType"); - assert(connectorErrorsType, () => `Expected "${connectorErrorsName}" to be defined`); - return connectorErrorsType; - } + type: (schema, feature) => + lookupFeatureTypeInSchema(CONNECTOR_ERRORS, 'InputObjectType', schema, feature) } ], // We "compose" these directives using the `@join__directive` mechanism, @@ -331,22 +311,15 @@ export class ConnectSpecDefinition extends FeatureDefinition { { name: 'http', type: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @connect spec"); - const sourceHttpName = feature.typeNameInSchema(SOURCE_HTTP); - const sourceHttpType = schema.typeOfKind(sourceHttpName, "InputObjectType"); - assert(sourceHttpType, () => `Expected "${sourceHttpName}" to be defined`); + const sourceHttpType = + lookupFeatureTypeInSchema(SOURCE_HTTP, 'InputObjectType', schema, feature); return new NonNullType(sourceHttpType); } }, { name: 'errors', - type: (schema, feature) => { - assert(feature, "Shouldn't be added without being attached to a @connect spec"); - const connectorErrorsName = feature.typeNameInSchema(CONNECTOR_ERRORS); - const connectorErrorsType = schema.typeOfKind(connectorErrorsName, "InputObjectType"); - assert(connectorErrorsType, () => `Expected "${connectorErrorsName}" to be defined`); - return connectorErrorsType; - } + type: (schema, feature) => + lookupFeatureTypeInSchema(CONNECTOR_ERRORS, 'InputObjectType', schema, feature) } ] }), From f406377622b2c959bf59e8f9e364ce26d4c5f673 Mon Sep 17 00:00:00 2001 From: "Sachin D. Shinde" Date: Mon, 6 Oct 2025 15:02:23 -0700 Subject: [PATCH 3/3] Implement one-off support for input objects in the connect spec (#3311) This PR implements one-off support for input objects in the connect spec. Note that this implementation is hacky, and has various edge cases where it won't behave as expected. In the short-term, we should avoid using input objects in specs. In the long-term, there's a proper strategy to supporting input objects in specs, which is described below. To give some background: 1. Our schema representation (`Schema`) uses JS singleton objects for schema elements. 2. Type references in these schema elements (e.g. the type of a field or argument) will refer directly to these JS singletons. 3. This schema representation also tracks type/directive references in the referenced schema element. This means that schema building has to proceed in a two-phase process. 1. The first phase declares the types/directive definitions/extensions (referred to as "shallow building" of the type/directive). 2. The second phase defines the types/directive definitions/extensions (referred to as just "building" of the type/directive). This is necessary because types/directives are allowed to form referential cycles, so we can't just e.g. topologically sort the types/directives by referential order and fully build in that order. A problem arises, however, with subgraph schemas (a.k.a. schemas using `FederationBlueprint`). In that case, schemas are allowed to omit definitions/extensions of spec types/directives, for both the `federation` and `connect` specs. These missing definitions/extensions are added (or if already added, they're checked) by the `checkOrAdd()` methods of type/directive specifications. The `checkOrAdd()` pattern has a design flaw in that it bundles shallow building, building, and checking into one single function. Specifically: 1. When building a schema, during the first phase, there may be missing spec types/directives that are not written. 2. This blocks us from the second phase of building types/directives, as they may depend on these missing types/directives. 3. This results in us calling `checkOrAdd()` for a type/directive spec to fill in the missing definitions. 4. However, `checkOrAdd()` for a type/directive specification, in bundling everything together, requires the types/directives it references to be fully built. This has several implications: 1. Specs with circular type/directive references just won't work. 2. Even if there's some topological order of references that can be established, that order is currently not computed or followed. The current `checkOrAdd()` calling logic just calls it for all registered type specifications, and then all registered directive specifications. This order has to be a topological referential order for schemas that omit all spec type/directive definitions/extensions to build properly, and that registration can be error-prone to manually specify/maintain. - Also, this isn't done in a way that's interleaved with the building of those elements if they already exist; for those elements, they need to be built prior to the `checkOrAdd()` calls, otherwise the checking part fails. 4. This has led to a very bespoke fragile pattern in schema building where we first fully build enums and scalars while skipping directive applications (since they can't reference other types/directives in that case), then directive definitions while skipping directive applications, and then call `checkOrAdd()` (after processing `@link`s). 5. This bespoke fragile pattern only worked before because the federation spec just has type/directive definitions that only reference enum/scalar types. And even in that case, if someone e.g. wrote in a spec directive definition that referenced spec enums/scalars, they must also write the definitions for those referenced enums/scalars, since otherwise that directive definition would fail to build. 6. This bespoke fragile pattern does not work for connectors, since those spec types/directives reference spec input object types, and some of those spec input object types even reference other spec input object types. In order to support connectors usage of input object types in the short-term, we must do a few things. 1. Verify there are no referential cycles (there aren't thankfully), and register types/directives in the appropriate topological order. 2. Before calling `checkOrAdd()` for the connect spec, ensure that input objects are built if they exist in the schema. This is going to have the same limitation that the federation spec has with directive definitions, where if someone wrote in a spec input object definition that references other spec types/directives, they also need to write the definitions for those referenced types/directives. This will handle the main cases we wish to support, which are when no connect spec definitions/extensions are provided and when they're all provided. That's what this PR does (it also adds a test). The actual long-term solution involves splitting the `checkOrAdd()` logic into three separate functions: 1. Shallow building of spec types/directives that aren't in the schema (accounting for renames as needed). 2. Full building of spec types/directives that aren't in the schema. 3. Checking of spec types/directives to ensure they conform to expected definitions. The gist is that we first do some processing on schema definitions/extensions to bootstrap the `link` spec, then we couple spec shallow building with the rest of shallow building (phase 1), couple spec full building with the rest of full building (phase 2), and save checking for after everything's built. There could be some additional tracking to avoid doing checking for types/directives that weren't originally in the schema, but that's not strictly necessary. This PR also implements various fixes for bugs noticed in https://github.com/apollographql/federation/pull/3310; these will be pointed out in that PR's review. --- .../src/__tests__/connectors.test.ts | 214 ++++++++++-------- internals-js/src/buildSchema.ts | 51 +++++ .../src/directiveAndTypeSpecification.ts | 62 +---- internals-js/src/specs/connectSpec.ts | 151 ++++++++++-- 4 files changed, 312 insertions(+), 166 deletions(-) diff --git a/composition-js/src/__tests__/connectors.test.ts b/composition-js/src/__tests__/connectors.test.ts index 06a984e88..5c9dd5db9 100644 --- a/composition-js/src/__tests__/connectors.test.ts +++ b/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); } }); diff --git a/internals-js/src/buildSchema.ts b/internals-js/src/buildSchema.ts index aeee08000..57db92eb3 100644 --- a/internals-js/src/buildSchema.ts +++ b/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(); + 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; diff --git a/internals-js/src/directiveAndTypeSpecification.ts b/internals-js/src/directiveAndTypeSpecification.ts index 586cfa277..c4eaa2761 100644 --- a/internals-js/src/directiveAndTypeSpecification.ts +++ b/internals-js/src/directiveAndTypeSpecification.ts @@ -3,10 +3,10 @@ import { ArgumentDefinition, CoreFeature, DirectiveDefinition, - EnumType, InputObjectType, + EnumType, InputType, isCustomScalarType, - isEnumType, isInputObjectType, + isEnumType, isListType, isNonNullType, isObjectType, @@ -257,62 +257,6 @@ export function createObjectTypeSpecification({ } } -export function createInputObjectTypeSpecification({ - name, - inputFieldsFct, -}: { - name: string, - inputFieldsFct: (schema: Schema, feature?: CoreFeature) => InputFieldSpecification[], -}): TypeSpecification { - return { - name, - checkOrAdd: (schema: Schema, feature?: CoreFeature, asBuiltIn?: boolean) => { - const actualName = feature?.typeNameInSchema(name) ?? name; - const expectedFields = inputFieldsFct(schema, feature); - const existing = schema.type(actualName); - if (existing) { - let errors = ensureSameTypeKind('InputObjectType', existing); - if (errors.length > 0) { - return errors; - } - assert(isInputObjectType(existing), 'Should be an input object type'); - for (const { name: field_name, type, defaultValue } of expectedFields) { - const existingField = existing.field(field_name); - if (!existingField) { - errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( - `Invalid definition of type ${name}: missing input field ${field_name}`, - { nodes: existing.sourceAST }, - )); - continue; - } - if (!sameType(type, existingField.type!)) { - errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( - `Invalid definition for field ${field_name} of type ${name}: should have type ${type} but found type ${existingField.type}`, - { nodes: existingField.sourceAST }, - )); - } - if (defaultValue !== existingField.defaultValue) { - errors = errors.concat(ERRORS.TYPE_DEFINITION_INVALID.err( - `Invalid definition for field ${field_name} of type ${name}: should have default value ${defaultValue} but found type ${existingField.defaultValue}`, - { nodes: existingField.sourceAST }, - )); - } - } - return errors; - } else { - const createdType = schema.addType(new InputObjectType(actualName, asBuiltIn)); - for (const { name, type, defaultValue } of expectedFields) { - const newField = createdType.addField(name, type); - if (defaultValue) { - newField.defaultValue = defaultValue; - } - } - return []; - } - }, - } -} - export function createUnionTypeSpecification({ name, membersFct, @@ -400,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 ? [] : [ diff --git a/internals-js/src/specs/connectSpec.ts b/internals-js/src/specs/connectSpec.ts index 627f59220..3537e27c7 100644 --- a/internals-js/src/specs/connectSpec.ts +++ b/internals-js/src/specs/connectSpec.ts @@ -9,15 +9,26 @@ import { import { CoreFeature, InputObjectType, - ListType, NamedType, - NonNullType, ScalarType, Schema, + isInputObjectType, + isNonNullType, + ListType, + NamedType, + NonNullType, + ScalarType, + Schema, } from '../definitions'; import { registerKnownFeature } from '../knownCoreFeatures'; import { - createDirectiveSpecification, createInputObjectTypeSpecification, + createDirectiveSpecification, createScalarTypeSpecification, + ensureSameTypeKind, + InputFieldSpecification, + TypeSpecification, } from '../directiveAndTypeSpecification'; -import {assert} from "../utils"; +import { ERRORS } from '../error'; +import { sameType } from '../types'; +import { assert } from '../utils'; +import { valueEquals, valueToString } from '../values'; export const connectIdentity = 'https://specs.apollo.dev/connect'; @@ -133,7 +144,7 @@ export class ConnectSpecDefinition extends FeatureDefinition { # added in v0.2 path: JSONSelection - query: JSONSelection + queryParams: JSONSelection } */ this.registerType( @@ -147,7 +158,7 @@ export class ConnectSpecDefinition extends FeatureDefinition { return [ { name: 'baseURL', - type: schema.stringType() + type: new NonNullType(schema.stringType()) }, { name: 'headers', @@ -178,7 +189,7 @@ export class ConnectSpecDefinition extends FeatureDefinition { # added in v0.2 path: JSONSelection - query: JSONSelection + queryParams: JSONSelection } */ this.registerType( @@ -238,9 +249,9 @@ export class ConnectSpecDefinition extends FeatureDefinition { source: String http: ConnectHTTP! batch: ConnectBatch + errors: ConnectorErrors selection: JSONSelection! entity: Boolean = false - errors: ConnectorErrors ) repeatable on FIELD_DEFINITION | OBJECT # added in v0.2, validation enforced in rust */ @@ -268,19 +279,22 @@ export class ConnectSpecDefinition extends FeatureDefinition { lookupFeatureTypeInSchema(CONNECT_BATCH, 'InputObjectType', schema, feature) }, { - name: 'selection', + name: 'errors', type: (schema, feature) => - lookupFeatureTypeInSchema(JSON_SELECTION, 'ScalarType', schema, feature) + lookupFeatureTypeInSchema(CONNECTOR_ERRORS, 'InputObjectType', schema, feature) + }, + { + name: 'selection', + type: (schema, feature) => { + const jsonSelectionType = + lookupFeatureTypeInSchema(JSON_SELECTION, 'ScalarType', schema, feature); + return new NonNullType(jsonSelectionType); + } }, { name: 'entity', type: (schema) => schema.booleanType(), defaultValue: false - }, - { - name: 'errors', - type: (schema, feature) => - lookupFeatureTypeInSchema(CONNECTOR_ERRORS, 'InputObjectType', schema, feature) } ], // We "compose" these directives using the `@join__directive` mechanism, @@ -293,7 +307,7 @@ export class ConnectSpecDefinition extends FeatureDefinition { /* directive @source( name: String! - http: SourceHttp! + http: SourceHTTP! errors: ConnectorErrors ) repeatable on SCHEMA */ @@ -348,3 +362,108 @@ export const CONNECT_VERSIONS = new FeatureDefinitions( ); registerKnownFeature(CONNECT_VERSIONS); + +// This function is purposefully declared only in this file and without export. +// +// Do NOT add this to "internals-js/src/directiveAndTypeSpecification.ts", and +// do NOT export this function. +// +// Subgraph schema building, at this time of writing, does not really support +// input objects in specs. We did a number of one-off things to support them in +// the connect spec's case, and it will be non-maintainable/bug-prone to do them +// again. +// +// There's work to be done to support input objects more generally; please see +// https://github.com/apollographql/federation/pull/3311 for more information. +function createInputObjectTypeSpecification({ + name, + inputFieldsFct, +}: { + name: string, + inputFieldsFct: (schema: Schema, feature?: CoreFeature) => InputFieldSpecification[], +}): TypeSpecification { + return { + name, + checkOrAdd: (schema: Schema, feature?: CoreFeature, asBuiltIn?: boolean) => { + const actualName = feature?.typeNameInSchema(name) ?? name; + const expectedFields = inputFieldsFct(schema, feature); + const existing = schema.type(actualName); + if (existing) { + let errors = ensureSameTypeKind('InputObjectType', existing); + if (errors.length > 0) { + return errors; + } + assert(isInputObjectType(existing), 'Should be an input object type'); + // The following mimics `ensureSameArguments()`, but with some changes. + for (const { name: fieldName, type, defaultValue } of expectedFields) { + const existingField = existing.field(fieldName); + if (!existingField) { + // Not declaring an optional input field is ok: that means you won't + // be able to pass a non-default value in your schema, but we allow + // you that. But missing a required input field it not ok. + if (isNonNullType(type) && defaultValue === undefined) { + errors.push(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for type ${name}: missing required input field "${fieldName}"`, + { nodes: existing.sourceAST }, + )); + } + continue; + } + + let existingType = existingField.type!; + if (isNonNullType(existingType) && !isNonNullType(type)) { + // It's ok to redefine an optional input field as mandatory. For + // instance, if you want to force people on your team to provide a + // "maxSize", you can redefine ConnectBatch as + // `input ConnectBatch { maxSize: Int! }` to get validation. In + // other words, you are allowed to always pass an input field that + // is optional if you so wish. + existingType = existingType.ofType; + } + // Note that while `ensureSameArguments()` allows input type + // redefinitions (e.g. allowing users to declare `String` instead of a + // custom scalar), this behavior can be confusing/error-prone more + // generally, so we forbid this for now. We can relax this later on a + // case-by-case basis if needed. + // + // Further, `ensureSameArguments()` would skip default value checking + // if the input type was non-nullable. It's unclear why this is there; + // it may have been a mistake due to the impression that non-nullable + // inputs can't have default values (they can), or this may have been + // to avoid some breaking change, but there's no such limitation in + // the case of input objects, so we always validate default values + // here. + if (!sameType(type, existingType)) { + errors.push(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for type ${name}: input field "${fieldName}" should have type "${type}" but found type "${existingField.type!}"`, + { nodes: existingField.sourceAST }, + )); + } else if (!valueEquals(defaultValue, existingField.defaultValue)) { + errors.push(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition type ${name}: input field "${fieldName}" should have default value ${valueToString(defaultValue)} but found default value ${valueToString(existingField.defaultValue)}`, + { nodes: existingField.sourceAST }, + )); + } + } + for (const existingField of existing.fields()) { + // If it's an expected input field, we already validated it. But we + // still need to reject unknown input fields. + if (!expectedFields.some((field) => field.name === existingField.name)) { + errors.push(ERRORS.TYPE_DEFINITION_INVALID.err( + `Invalid definition for type ${name}: unknown/unsupported input field "${existingField.name}"`, + { nodes: existingField.sourceAST }, + )); + } + } + return errors; + } else { + const createdType = schema.addType(new InputObjectType(actualName, asBuiltIn)); + for (const { name, type, defaultValue } of expectedFields) { + const newField = createdType.addField(name, type); + newField.defaultValue = defaultValue; + } + return []; + } + }, + } +}