diff --git a/packages/apollo-federation/CHANGELOG.md b/packages/apollo-federation/CHANGELOG.md index 87c43bc9b62..77affcda5dc 100644 --- a/packages/apollo-federation/CHANGELOG.md +++ b/packages/apollo-federation/CHANGELOG.md @@ -4,6 +4,8 @@ > 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 the appropriate changes within that release will be moved into the new section. +* Strip all Type System Directives during composition [#3736](https://github.com/apollographql/apollo-server/pull/3736) + # v0.11.1 > [See complete versioning details.](https://github.com/apollographql/apollo-server/commit/2a4c654986a158aaccf947ee56a4bfc48a3173c7) diff --git a/packages/apollo-federation/src/composition/__tests__/composeAndValidate.test.ts b/packages/apollo-federation/src/composition/__tests__/composeAndValidate.test.ts index 9980434dade..f3e20cec8bf 100644 --- a/packages/apollo-federation/src/composition/__tests__/composeAndValidate.test.ts +++ b/packages/apollo-federation/src/composition/__tests__/composeAndValidate.test.ts @@ -329,11 +329,11 @@ describe('composition of value types', () => { `union CatalogItem = Couch | Mattress`, ); expect(schema.getType('Couch')).toMatchInlineSnapshot(` - type Couch { - sku: ID! - material: String! - } - `); + type Couch { + sku: ID! + material: String! + } + `); }); it('input types', () => { @@ -345,11 +345,11 @@ describe('composition of value types', () => { `); expect(errors).toHaveLength(0); expect(schema.getType('NewProductInput')).toMatchInlineSnapshot(` - input NewProductInput { - sku: ID! - type: String - } - `); + input NewProductInput { + sku: ID! + type: String + } + `); }); it('interfaces', () => { @@ -360,10 +360,10 @@ describe('composition of value types', () => { `); expect(errors).toHaveLength(0); expect(schema.getType('Product')).toMatchInlineSnapshot(` - interface Product { - sku: ID! - } - `); + interface Product { + sku: ID! + } + `); }); it('enums', () => { @@ -375,11 +375,11 @@ describe('composition of value types', () => { `); expect(errors).toHaveLength(0); expect(schema.getType('CatalogItemEnum')).toMatchInlineSnapshot(` - enum CatalogItemEnum { - COUCH - MATTRESS - } - `); + enum CatalogItemEnum { + COUCH + MATTRESS + } + `); }); }); @@ -551,3 +551,76 @@ describe('composition of value types', () => { }); }); }); + +describe('composition of schemas with directives', () => { + /** + * To see which usage sites indicate whether a directive is "executable" or + * merely for use by the type-system ("type-system"), see the GraphQL spec: + * https://graphql.github.io/graphql-spec/June2018/#sec-Type-System.Directives + */ + it('preserves executable and purges type-system directives', () => { + const serviceA = { + typeDefs: gql` + "directives at FIELDs are executable" + directive @audit(risk: Int!) on FIELD + + "directives at FIELD_DEFINITIONs are for the type-system" + directive @transparency(concealment: Int!) on FIELD_DEFINITION + + type EarthConcern { + environmental: String! @transparency(concealment: 5) + } + + extend type Query { + importantDirectives: [EarthConcern!]! + } + `, + name: 'serviceA', + }; + + const serviceB = { + typeDefs: gql` + "directives at FIELDs are executable" + directive @audit(risk: Int!) on FIELD + + "directives at FIELD_DEFINITIONs are for the type-system" + directive @transparency(concealment: Int!) on FIELD_DEFINITION + + "directives at OBJECTs are for the type-system" + directive @experimental on OBJECT + + extend type EarthConcern @experimental { + societal: String! @transparency(concealment: 6) + } + `, + name: 'serviceB', + }; + + const { schema, errors } = composeAndValidate([serviceA, serviceB]); + expect(errors).toHaveLength(0); + + const audit = schema.getDirective('audit'); + expect(audit).toMatchInlineSnapshot(`"@audit"`); + + const transparency = schema.getDirective('transparency'); + expect(transparency).toBeUndefined(); + + const type = schema.getType('EarthConcern') as GraphQLObjectType; + + expect(type.astNode).toMatchInlineSnapshot(` + type EarthConcern { + environmental: String! + } + `); + + const fields = type.getFields(); + + expect(fields['environmental'].astNode).toMatchInlineSnapshot( + `environmental: String!`, + ); + + expect(fields['societal'].astNode).toMatchInlineSnapshot( + `societal: String!`, + ); + }); +}); diff --git a/packages/apollo-federation/src/composition/compose.ts b/packages/apollo-federation/src/composition/compose.ts index 50b8d19e86d..d88d087be0e 100644 --- a/packages/apollo-federation/src/composition/compose.ts +++ b/packages/apollo-federation/src/composition/compose.ts @@ -29,6 +29,7 @@ import { mapValues, isFederationDirective, executableDirectiveLocations, + stripTypeSystemDirectivesFromTypeDefs, } from './utils'; import { ServiceDefinition, @@ -135,7 +136,14 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) { externalFields.push(...strippedFields); - for (let definition of typeDefsWithoutExternalFields.definitions) { + // Type system directives from downstream services are not a concern of the + // gateway, but rather the services on which the fields live which serve + // those types. In other words, its up to an implementing service to + // act on such directives, not the gateway. + const typeDefsWithoutTypeSystemDirectives = + stripTypeSystemDirectivesFromTypeDefs(typeDefsWithoutExternalFields); + + for (const definition of typeDefsWithoutTypeSystemDirectives.definitions) { if ( definition.kind === Kind.OBJECT_TYPE_DEFINITION || definition.kind === Kind.OBJECT_TYPE_EXTENSION diff --git a/packages/apollo-federation/src/composition/utils.ts b/packages/apollo-federation/src/composition/utils.ts index a70bd812de9..34ca9b06aa8 100644 --- a/packages/apollo-federation/src/composition/utils.ts +++ b/packages/apollo-federation/src/composition/utils.ts @@ -83,6 +83,20 @@ export function stripExternalFieldsFromTypeDefs( return { typeDefsWithoutExternalFields, strippedFields }; } +export function stripTypeSystemDirectivesFromTypeDefs(typeDefs: DocumentNode) { + const typeDefsWithoutTypeSystemDirectives = visit(typeDefs, { + Directive(node) { + const isFederationDirective = federationDirectives.some( + ({ name }) => name === node.name.value, + ); + // Returning `null` to a visit will cause it to be removed from the tree. + return isFederationDirective ? undefined : null; + }, + }) as DocumentNode; + + return typeDefsWithoutTypeSystemDirectives; +} + /** * Returns a closure that strips fields marked with `@external` and adds them * to an array.