diff --git a/docs/source/errors.md b/docs/source/errors.md index d4d67d77c..c7f40142c 100644 --- a/docs/source/errors.md +++ b/docs/source/errors.md @@ -53,6 +53,13 @@ If Apollo Gateway encounters an error, composition fails. This document lists co | `REQUIRES_FIELDS_MISSING_ON_BASE` | The `fields` argument of an entity field's `@requires` directive includes a field that is not defined in the entity's originating subgraph.`| | `REQUIRES_USED_ON_BASE` | An entity field is marked with `@requires` in the entity's originating subgraph, which is invalid. | +## Non-federation directives + +| Code | Description | +|---|---| +| `TAG_USED_WITH_EXTERNAL` | Fields marked as `@external` cannot also have `@tag` usages. | +| `INACCESSIBLE_USED_WITH_EXTERNAL` | Fields marked as `@external` cannot also have `@inaccessible` usages. | + ## Custom directives | Code | Description | diff --git a/federation-integration-testsuite-js/src/fixtures/reviews.ts b/federation-integration-testsuite-js/src/fixtures/reviews.ts index ccf3399d0..7660773bc 100644 --- a/federation-integration-testsuite-js/src/fixtures/reviews.ts +++ b/federation-integration-testsuite-js/src/fixtures/reviews.ts @@ -29,7 +29,7 @@ export const typeDefs = gql` } extend type User @key(fields: "id") { - id: ID! @external @tag(name: "reviews") @inaccessible + id: ID! @external username: String @external reviews: [Review] numberOfReviews: Int! diff --git a/federation-js/CHANGELOG.md b/federation-js/CHANGELOG.md index facc66ea9..5f2c8b538 100644 --- a/federation-js/CHANGELOG.md +++ b/federation-js/CHANGELOG.md @@ -5,6 +5,7 @@ > The changes noted within this `vNEXT` section have not been released yet. New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development. When a release is being prepared, a new header will be (manually) created below and the appropriate changes within that release will be moved into the new section. - Skip missing types while iterating over field directive usages. It's possible to capture directive usages on fields whose types don't actually exist in the schema (due to invalid composition). See PR for more details. [PR #868](https://github.com/apollographql/federation/pull/868) +- Prevent the usage of either @tag or @inaccessible on fields that are marked as @external. [PR #869](https://github.com/apollographql/federation/pull/869) ## v0.26.0 diff --git a/federation-js/src/composition/__tests__/composeAndValidate.test.ts b/federation-js/src/composition/__tests__/composeAndValidate.test.ts index 56e216341..d907ba14b 100644 --- a/federation-js/src/composition/__tests__/composeAndValidate.test.ts +++ b/federation-js/src/composition/__tests__/composeAndValidate.test.ts @@ -185,14 +185,14 @@ describe('unknown types', () => { assertCompositionFailure(compositionResult); expect(compositionResult.errors[0]).toMatchInlineSnapshot(` Object { - "code": "EXTENSION_WITH_NO_BASE", + "code": "TAG_USED_WITH_EXTERNAL", "locations": Array [ Object { - "column": 1, - "line": 2, + "column": 21, + "line": 3, }, ], - "message": "[inventory] Product -> \`Product\` is an extension type, but \`Product\` is not defined in any service", + "message": "[inventory] Product.id -> Found illegal use of @tag directive. @tag directives cannot currently be used in tandem with an @external directive.", } `); }); diff --git a/federation-js/src/composition/compose.ts b/federation-js/src/composition/compose.ts index e2e82d579..f609edf64 100644 --- a/federation-js/src/composition/compose.ts +++ b/federation-js/src/composition/compose.ts @@ -21,7 +21,10 @@ import { DirectiveNode, } from 'graphql'; import { transformSchema } from 'apollo-graphql'; -import apolloTypeSystemDirectives, { appliedDirectives, federationDirectives } from '../directives'; +import apolloTypeSystemDirectives, { + otherKnownDirectiveDefinitions, + federationDirectives, +} from '../directives'; import { findDirectivesOnNode, isStringValueNode, @@ -130,9 +133,10 @@ type FieldDirectivesMap = Map; type TypeNameToFieldDirectivesMap = Map; /** - * A set of directive names that have been used at least once + * A set of directive names that have been used at least once - specifically + * non-federation, Apollo-specific directives */ -type AppliedDirectiveUsages = Set; +type OtherKnownDirectiveUsages = Set; /** * Loop over each service and process its typeDefs (`definitions`) @@ -148,7 +152,7 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) { const keyDirectivesMap: KeyDirectivesMap = Object.create(null); const valueTypes: ValueTypes = new Set(); const typeNameToFieldDirectivesMap: TypeNameToFieldDirectivesMap = new Map(); - const appliedDirectiveUsages: AppliedDirectiveUsages = new Set(); + const otherKnownDirectiveUsages: OtherKnownDirectiveUsages = new Set(); for (const { typeDefs, name: serviceName } of serviceList) { // Build a new SDL with @external fields removed, as well as information about @@ -192,19 +196,15 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) { } } - // Capture `@tag` and `@inaccessible` directive applications + // Capture `@tag` and `@inaccessible` directive usages for (const field of definition.fields ?? []) { const fieldName = field.name.value; - const tagUsages = findDirectivesOnNode(field, 'tag'); - const inaccessibleUsages = findDirectivesOnNode( - field, - 'inaccessible', - ); + const inaccessibleUsages = findDirectivesOnNode(field, 'inaccessible'); - if (tagUsages.length > 0) appliedDirectiveUsages.add('tag'); + if (tagUsages.length > 0) otherKnownDirectiveUsages.add('tag'); if (inaccessibleUsages.length > 0) - appliedDirectiveUsages.add('inaccessible'); + otherKnownDirectiveUsages.add('inaccessible'); if (tagUsages.length > 0 || inaccessibleUsages.length > 0) { const fieldToDirectivesMap = mapGetOrSet( @@ -352,31 +352,6 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) { } } - // We need to capture applied directives from the external fields as well, - // which are stripped and excluded from the main loop over the typeDefs - for (const { parentTypeName, field } of externalFields) { - const tagDirectivesOnField = findDirectivesOnNode(field, 'tag'); - const inaccessibleDirectivesOnField = findDirectivesOnNode(field, 'inaccessible'); - - const appliedDirectivesOnField = [ - ...tagDirectivesOnField, - ...inaccessibleDirectivesOnField, - ]; - if (appliedDirectivesOnField.length > 0) { - const fieldToDirectivesMap = mapGetOrSet( - typeNameToFieldDirectivesMap, - parentTypeName, - new Map(), - ); - const directives = mapGetOrSet( - fieldToDirectivesMap, - field.name.value, - [], - ); - directives.push(...appliedDirectivesOnField); - } - } - // Since all Query/Mutation definitions in service schemas are treated as // extensions, we don't have a Query or Mutation DEFINITION in the definitions // list. Without a Query/Mutation definition, we can't _extend_ the type. @@ -397,7 +372,7 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) { keyDirectivesMap, valueTypes, typeNameToFieldDirectivesMap, - appliedDirectiveUsages, + otherKnownDirectiveUsages, }; } @@ -405,27 +380,28 @@ export function buildSchemaFromDefinitionsAndExtensions({ typeDefinitionsMap, typeExtensionsMap, directiveDefinitionsMap, - appliedDirectiveUsages, + otherKnownDirectiveUsages, }: { typeDefinitionsMap: TypeDefinitionsMap; typeExtensionsMap: TypeExtensionsMap; directiveDefinitionsMap: DirectiveDefinitionsMap; - appliedDirectiveUsages: AppliedDirectiveUsages; + otherKnownDirectiveUsages: OtherKnownDirectiveUsages; }) { let errors: GraphQLError[] | undefined = undefined; - // We only want to include the definitions of applied directives (currently - // just @tag and @include) if there are usages. - const appliedDirectivesToInclude = appliedDirectives.filter((directive) => - appliedDirectiveUsages.has(directive.name), - ); + // We only want to include the definitions of other known directives (currently + // just `@tag` and `@include`) if there are usages. + const otherKnownDirectiveDefinitionsToInclude = + otherKnownDirectiveDefinitions.filter((directive) => + otherKnownDirectiveUsages.has(directive.name), + ); let schema = new GraphQLSchema({ query: undefined, directives: [ ...specifiedDirectives, ...federationDirectives, - ...appliedDirectivesToInclude, + ...otherKnownDirectiveDefinitionsToInclude, ], }); @@ -686,25 +662,29 @@ export function addFederationMetadataToSchemaNodes({ for (const [ fieldName, - appliedDirectives, + otherKnownDirectiveUsages, ] of fieldsToDirectivesMap.entries()) { const field = type.getFields()[fieldName]; const seenNonRepeatableDirectives: Record = {}; - const filteredDirectives = appliedDirectives.filter(directive => { - const name = directive.name.value; - const matchingDirective = apolloTypeSystemDirectives.find(d => d.name === name); - if (matchingDirective?.isRepeatable) return true; - if (seenNonRepeatableDirectives[name]) return false; - seenNonRepeatableDirectives[name] = true; - return true; - }); + const filteredDirectives = otherKnownDirectiveUsages.filter( + (directive) => { + const name = directive.name.value; + const matchingDirective = apolloTypeSystemDirectives.find( + (d) => d.name === name, + ); + if (matchingDirective?.isRepeatable) return true; + if (seenNonRepeatableDirectives[name]) return false; + seenNonRepeatableDirectives[name] = true; + return true; + }, + ); // TODO: probably don't need to recreate these objects const existingMetadata = getFederationMetadata(field); const fieldFederationMetadata: FederationField = { ...existingMetadata, - appliedDirectives: filteredDirectives, + otherKnownDirectiveUsages: filteredDirectives, }; field.extensions = { @@ -726,14 +706,14 @@ export function composeServices(services: ServiceDefinition[]): CompositionResul keyDirectivesMap, valueTypes, typeNameToFieldDirectivesMap, - appliedDirectiveUsages, + otherKnownDirectiveUsages, } = buildMapsFromServiceList(services); let { schema, errors } = buildSchemaFromDefinitionsAndExtensions({ typeDefinitionsMap, typeExtensionsMap, directiveDefinitionsMap, - appliedDirectiveUsages, + otherKnownDirectiveUsages, }); // TODO: We should fix this to take non-default operation root types in diff --git a/federation-js/src/composition/types.ts b/federation-js/src/composition/types.ts index 7e65bc0ed..cc39841f3 100644 --- a/federation-js/src/composition/types.ts +++ b/federation-js/src/composition/types.ts @@ -39,7 +39,7 @@ export interface FederationField { requires?: ReadonlyArray; provides?: ReadonlyArray; belongsToValueType?: boolean; - appliedDirectives?: DirectiveNode[] + otherKnownDirectiveUsages?: DirectiveNode[] } export interface FederationDirective { diff --git a/federation-js/src/composition/validate/preComposition/__tests__/tagOrInaccessibleUsedWithExternal.test.ts b/federation-js/src/composition/validate/preComposition/__tests__/tagOrInaccessibleUsedWithExternal.test.ts new file mode 100644 index 000000000..248e8a416 --- /dev/null +++ b/federation-js/src/composition/validate/preComposition/__tests__/tagOrInaccessibleUsedWithExternal.test.ts @@ -0,0 +1,173 @@ +import { tagOrInaccessibleUsedWithExternal } from '..'; +import { + gql, + graphqlErrorSerializer, +} from 'apollo-federation-integration-testsuite'; + +expect.addSnapshotSerializer(graphqlErrorSerializer); + +describe('tagOrInaccessibleUsedWithExternal', () => { + describe('no errors', () => { + it('has no errors @external and @tag are used separately', () => { + const serviceA = { + typeDefs: gql` + type Query { + product: Product @tag(name: "prod") + } + + extend type Product @key(fields: "sku") { + sku: String @external + } + `, + name: 'serviceA', + }; + + const errors = tagOrInaccessibleUsedWithExternal(serviceA); + expect(errors).toHaveLength(0); + }); + + it('has no errors @external and @inaccessible are used separately', () => { + const serviceA = { + typeDefs: gql` + type Query { + product: Product @inaccessible + } + + extend type Product @key(fields: "sku") { + sku: String @external + } + `, + name: 'serviceA', + }; + + const errors = tagOrInaccessibleUsedWithExternal(serviceA); + expect(errors).toHaveLength(0); + }); + }); + + describe('errors', () => { + it('errors when `@external` and `@tag` are used on the same field of an object extension', () => { + const serviceA = { + typeDefs: gql` + type Query { + product: Product @tag(name: "prod") + } + + extend type Product @key(fields: "sku") { + sku: String @external @tag(name: "prod") + } + `, + name: 'serviceA', + }; + + const errors = tagOrInaccessibleUsedWithExternal(serviceA); + expect(errors).toMatchInlineSnapshot(` + Array [ + Object { + "code": "TAG_USED_WITH_EXTERNAL", + "locations": Array [ + Object { + "column": 25, + "line": 7, + }, + ], + "message": "[serviceA] Product.sku -> Found illegal use of @tag directive. @tag directives cannot currently be used in tandem with an @external directive.", + }, + ] + `); + }); + + it('errors when `@external` and `@tag` are used on the same field of an interface extension', () => { + const serviceA = { + typeDefs: gql` + type Query { + product: Product @tag(name: "prod") + } + + extend interface Product @key(fields: "sku") { + sku: String @external @tag(name: "prod") + } + `, + name: 'serviceA', + }; + + const errors = tagOrInaccessibleUsedWithExternal(serviceA); + expect(errors).toMatchInlineSnapshot(` + Array [ + Object { + "code": "TAG_USED_WITH_EXTERNAL", + "locations": Array [ + Object { + "column": 25, + "line": 7, + }, + ], + "message": "[serviceA] Product.sku -> Found illegal use of @tag directive. @tag directives cannot currently be used in tandem with an @external directive.", + }, + ] + `); + }); + + it('errors when `@external` and `@inaccessible` are used on the same field of an object extension', () => { + const serviceA = { + typeDefs: gql` + type Query { + product: Product @tag(name: "prod") + } + + extend type Product @key(fields: "sku") { + sku: String @external @inaccessible + } + `, + name: 'serviceA', + }; + + const errors = tagOrInaccessibleUsedWithExternal(serviceA); + expect(errors).toMatchInlineSnapshot(` + Array [ + Object { + "code": "INACCESSIBLE_USED_WITH_EXTERNAL", + "locations": Array [ + Object { + "column": 25, + "line": 7, + }, + ], + "message": "[serviceA] Product.sku -> Found illegal use of @inaccessible directive. @inaccessible directives cannot currently be used in tandem with an @external directive.", + }, + ] + `); + }); + + it('errors when `@external` and `@inaccessible` are used on the same field of an interface extension', () => { + const serviceA = { + typeDefs: gql` + type Query { + product: Product @tag(name: "prod") + } + + extend interface Product @key(fields: "sku") { + sku: String @external @inaccessible + } + `, + name: 'serviceA', + }; + + const errors = tagOrInaccessibleUsedWithExternal(serviceA); + expect(errors).toMatchInlineSnapshot(` + Array [ + Object { + "code": "INACCESSIBLE_USED_WITH_EXTERNAL", + "locations": Array [ + Object { + "column": 25, + "line": 7, + }, + ], + "message": "[serviceA] Product.sku -> Found illegal use of @inaccessible directive. @inaccessible directives cannot currently be used in tandem with an @external directive.", + }, + ] + `); + }); + }); +}); diff --git a/federation-js/src/composition/validate/preComposition/index.ts b/federation-js/src/composition/validate/preComposition/index.ts index c2d78e45b..6d0d1e4dc 100644 --- a/federation-js/src/composition/validate/preComposition/index.ts +++ b/federation-js/src/composition/validate/preComposition/index.ts @@ -4,3 +4,4 @@ export { keyFieldsMissingExternal } from './keyFieldsMissingExternal'; export { reservedFieldUsed } from './reservedFieldUsed'; export { duplicateEnumOrScalar } from './duplicateEnumOrScalar'; export { duplicateEnumValue } from './duplicateEnumValue'; +export { tagOrInaccessibleUsedWithExternal } from './tagOrInaccessibleUsedWithExternal'; diff --git a/federation-js/src/composition/validate/preComposition/tagOrInaccessibleUsedWithExternal.ts b/federation-js/src/composition/validate/preComposition/tagOrInaccessibleUsedWithExternal.ts new file mode 100644 index 000000000..815b39f59 --- /dev/null +++ b/federation-js/src/composition/validate/preComposition/tagOrInaccessibleUsedWithExternal.ts @@ -0,0 +1,78 @@ +import { + visit, + GraphQLError, + ObjectTypeExtensionNode, + InterfaceTypeExtensionNode, +} from 'graphql'; +import { ServiceDefinition } from '../../types'; + +import { + logServiceAndType, + errorWithCode, + findDirectivesOnNode, +} from '../../utils'; + +/** + * Ensure there are no fields with @external and _either_ @tag or @inaccessible. + * We're only concerned with the xTypeExtension nodes because @external is + * already disallowed on base types. + */ +export const tagOrInaccessibleUsedWithExternal = ({ + name: serviceName, + typeDefs, +}: ServiceDefinition) => { + const errors: GraphQLError[] = []; + + visit(typeDefs, { + ObjectTypeExtension: fieldsVisitor, + InterfaceTypeExtension: fieldsVisitor, + }); + + function fieldsVisitor( + typeDefinition: ObjectTypeExtensionNode | InterfaceTypeExtensionNode, + ) { + if (!typeDefinition.fields) return; + for (const fieldDefinition of typeDefinition.fields) { + const tagDirectives = findDirectivesOnNode(fieldDefinition, 'tag'); + const hasTagDirective = tagDirectives.length > 0; + const inaccessibleDirectives = findDirectivesOnNode( + fieldDefinition, + 'inaccessible', + ); + const hasInaccessibleDirective = inaccessibleDirectives.length > 0; + const hasExternalDirective = + findDirectivesOnNode(fieldDefinition, 'external').length > 0; + if (hasTagDirective && hasExternalDirective) { + errors.push( + errorWithCode( + 'TAG_USED_WITH_EXTERNAL', + logServiceAndType( + serviceName, + typeDefinition.name.value, + fieldDefinition.name.value, + ) + + `Found illegal use of @tag directive. @tag directives cannot currently be used in tandem with an @external directive.`, + tagDirectives, + ), + ); + } + + if (hasInaccessibleDirective && hasExternalDirective) { + errors.push( + errorWithCode( + 'INACCESSIBLE_USED_WITH_EXTERNAL', + logServiceAndType( + serviceName, + typeDefinition.name.value, + fieldDefinition.name.value, + ) + + `Found illegal use of @inaccessible directive. @inaccessible directives cannot currently be used in tandem with an @external directive.`, + inaccessibleDirectives, + ), + ); + } + } + } + + return errors; +}; diff --git a/federation-js/src/directives.ts b/federation-js/src/directives.ts index bce0eb356..5c6d110d1 100644 --- a/federation-js/src/directives.ts +++ b/federation-js/src/directives.ts @@ -80,11 +80,14 @@ export const federationDirectives = [ ProvidesDirective, ]; -export const appliedDirectives = [InaccessibleDirective, TagDirective]; +export const otherKnownDirectiveDefinitions = [ + InaccessibleDirective, + TagDirective, +]; const apolloTypeSystemDirectives = [ ...federationDirectives, - ...appliedDirectives, + ...otherKnownDirectiveDefinitions, ]; export default apolloTypeSystemDirectives; diff --git a/federation-js/src/service/__tests__/printFederatedSchema.test.ts b/federation-js/src/service/__tests__/printFederatedSchema.test.ts index c5e5eb9af..3872c17f9 100644 --- a/federation-js/src/service/__tests__/printFederatedSchema.test.ts +++ b/federation-js/src/service/__tests__/printFederatedSchema.test.ts @@ -186,7 +186,7 @@ describe('printFederatedSchema', () => { birthDate(locale: String): String @tag(name: \\"admin\\") @tag(name: \\"dev\\") goodAddress: Boolean @requires(fields: \\"metadata { address }\\") goodDescription: Boolean @requires(fields: \\"metadata { description }\\") - id: ID! @inaccessible @tag(name: \\"accounts\\") @tag(name: \\"reviews\\") + id: ID! @inaccessible @tag(name: \\"accounts\\") metadata: [UserMetadata] name: Name numberOfReviews: Int! diff --git a/federation-js/src/service/__tests__/printSupergraphSdl.test.ts b/federation-js/src/service/__tests__/printSupergraphSdl.test.ts index 72e94a754..3c05e8c35 100644 --- a/federation-js/src/service/__tests__/printSupergraphSdl.test.ts +++ b/federation-js/src/service/__tests__/printSupergraphSdl.test.ts @@ -273,7 +273,7 @@ describe('printSupergraphSdl', () => { birthDate(locale: String): String @join__field(graph: ACCOUNTS) @tag(name: \\"admin\\") @tag(name: \\"dev\\") goodAddress: Boolean @join__field(graph: REVIEWS, requires: \\"metadata{address}\\") goodDescription: Boolean @join__field(graph: INVENTORY, requires: \\"metadata{description}\\") - id: ID! @join__field(graph: ACCOUNTS) @inaccessible @tag(name: \\"accounts\\") @tag(name: \\"reviews\\") + id: ID! @join__field(graph: ACCOUNTS) @inaccessible @tag(name: \\"accounts\\") metadata: [UserMetadata] @join__field(graph: ACCOUNTS) name: Name @join__field(graph: ACCOUNTS) numberOfReviews: Int! @join__field(graph: REVIEWS) diff --git a/federation-js/src/service/printFederatedSchema.ts b/federation-js/src/service/printFederatedSchema.ts index baff86c65..090c69f59 100644 --- a/federation-js/src/service/printFederatedSchema.ts +++ b/federation-js/src/service/printFederatedSchema.ts @@ -214,7 +214,7 @@ function printObject(type: GraphQLObjectType, options?: Options): string { (isExtension ? 'extend ' : '') + `type ${type.name}${implementedInterfaces}` + // Federation addition for printing @key usages - printFederationDirectives(type) + + printFederationDirectiveUsages(type) + printFields(options, type) ); } @@ -233,7 +233,7 @@ function printInterface(type: GraphQLInterfaceType, options?: Options): string { `interface ${type.name}` + // Federation change: graphql@14 doesn't support interfaces implementing interfaces // printImplementedInterfaces(type) + - printFederationDirectives(type) + + printFederationDirectiveUsages(type) + printFields(options, type) ); } @@ -283,14 +283,14 @@ function printFields( ': ' + String(f.type) + printDeprecated(f) + - printFederationDirectives(f) + - printAppliedDirectives(f), + printFederationDirectiveUsages(f) + + printOtherKnownDirectiveUsages(f), ); return printBlock(fields); } // Federation change: *do* print the usages of federation directives. -function printFederationDirectives( +function printFederationDirectiveUsages( typeOrField: GraphQLNamedType | GraphQLField, ): string { if (!typeOrField.astNode) return ''; @@ -308,13 +308,13 @@ function printFederationDirectives( // Core addition: print `@tag` and `@inaccessible` directives found in subgraph // SDL into the supergraph SDL -function printAppliedDirectives(field: GraphQLField) { - const appliedDirectives = ( - field.extensions?.federation?.appliedDirectives ?? [] +function printOtherKnownDirectiveUsages(field: GraphQLField) { + const otherKnownDirectiveUsages = ( + field.extensions?.federation?.otherKnownDirectiveUsages ?? [] ) as DirectiveNode[]; - if (appliedDirectives.length < 1) return ''; - return ` ${appliedDirectives + if (otherKnownDirectiveUsages.length < 1) return ''; + return ` ${otherKnownDirectiveUsages .slice() .sort((a, b) => a.name.value.localeCompare(b.name.value)) .map(print) diff --git a/federation-js/src/service/printSupergraphSdl.ts b/federation-js/src/service/printSupergraphSdl.ts index 031ce838f..e6eff7089 100644 --- a/federation-js/src/service/printSupergraphSdl.ts +++ b/federation-js/src/service/printSupergraphSdl.ts @@ -33,7 +33,7 @@ import { assert } from '../utilities'; import { CoreDirective } from '../coreSpec'; import { getJoinDefinitions } from '../joinSpec'; import { printFieldSet } from '../composition/utils'; -import { appliedDirectives } from '../directives'; +import { otherKnownDirectiveDefinitions } from '../directives'; type Options = { /** @@ -172,19 +172,22 @@ function printSchemaDefinition(schema: GraphQLSchema): string { } function printCoreDirectives(schema: GraphQLSchema) { - const appliedDirectiveNames = appliedDirectives.map(({name}) => name); - const schemaDirectiveNames = schema.getDirectives().map(({ name }) => name); - const appliedDirectivesToInclude = schemaDirectiveNames.filter((name) => - appliedDirectiveNames.includes(name), + const otherKnownDirectiveNames = otherKnownDirectiveDefinitions.map( + ({ name }) => name, ); - const appliedDirectiveSpecUrls = appliedDirectivesToInclude.map( - (name) => `https://specs.apollo.dev/${name}/v0.1`, + const schemaDirectiveNames = schema.getDirectives().map(({ name }) => name); + const otherKnownDirectiveDefinitionsToInclude = schemaDirectiveNames.filter( + (name) => otherKnownDirectiveNames.includes(name), ); + const otherKnownDirectiveSpecUrls = + otherKnownDirectiveDefinitionsToInclude.map( + (name) => `https://specs.apollo.dev/${name}/v0.1`, + ); return [ 'https://specs.apollo.dev/core/v0.1', 'https://specs.apollo.dev/join/v0.1', - ...appliedDirectiveSpecUrls, + ...otherKnownDirectiveSpecUrls, ].map((feature) => `\n @core(feature: ${printStringLiteral(feature)})`); } @@ -367,7 +370,7 @@ function printFields( // We don't want to print field owner directives on fields belonging to an interface type (isObjectType(type) ? printJoinFieldDirectives(f, type, context) + - printAppliedDirectives(f) + printOtherKnownDirectiveUsages(f) : ''), ); @@ -440,13 +443,13 @@ function printJoinFieldDirectives( // Core addition: print `@tag` and `@inaccessible` directives found in subgraph // SDL into the supergraph SDL -function printAppliedDirectives(field: GraphQLField) { - const appliedDirectives = ( - field.extensions?.federation?.appliedDirectives ?? [] +function printOtherKnownDirectiveUsages(field: GraphQLField) { + const otherKnownDirectiveUsages = ( + field.extensions?.federation?.otherKnownDirectiveUsages ?? [] ) as DirectiveNode[]; - if (appliedDirectives.length < 1) return ''; - return ` ${appliedDirectives + if (otherKnownDirectiveUsages.length < 1) return ''; + return ` ${otherKnownDirectiveUsages .slice() .sort((a, b) => a.name.value.localeCompare(b.name.value)) .map(print) diff --git a/gateway-js/src/__tests__/loadSupergraphSdlFromStorage.test.ts b/gateway-js/src/__tests__/loadSupergraphSdlFromStorage.test.ts index 9eb11ea13..bf31fc64e 100644 --- a/gateway-js/src/__tests__/loadSupergraphSdlFromStorage.test.ts +++ b/gateway-js/src/__tests__/loadSupergraphSdlFromStorage.test.ts @@ -280,7 +280,7 @@ describe('loadSupergraphSdlFromStorage', () => { birthDate(locale: String): String @join__field(graph: ACCOUNTS) @tag(name: \\"admin\\") @tag(name: \\"dev\\") goodAddress: Boolean @join__field(graph: REVIEWS, requires: \\"metadata{address}\\") goodDescription: Boolean @join__field(graph: INVENTORY, requires: \\"metadata{description}\\") - id: ID! @join__field(graph: ACCOUNTS) @inaccessible @tag(name: \\"accounts\\") @tag(name: \\"reviews\\") + id: ID! @join__field(graph: ACCOUNTS) @inaccessible @tag(name: \\"accounts\\") metadata: [UserMetadata] @join__field(graph: ACCOUNTS) name: Name @join__field(graph: ACCOUNTS) numberOfReviews: Int! @join__field(graph: REVIEWS) diff --git a/query-planner-js/src/composedSchema/metadata.ts b/query-planner-js/src/composedSchema/metadata.ts index ed22c5aa4..706a921b2 100644 --- a/query-planner-js/src/composedSchema/metadata.ts +++ b/query-planner-js/src/composedSchema/metadata.ts @@ -73,5 +73,5 @@ export interface FederationFieldMetadata { graphName?: GraphName; requires?: FieldSet; provides?: FieldSet; - appliedDirectives?: DirectiveNode[]; + otherKnownDirectiveUsages?: DirectiveNode[]; }