From 3d51e584ddb027cb3ea22f406db3f25d6f6f2663 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 13 Jul 2021 13:07:31 -0700 Subject: [PATCH] Also validate against improper @inaccessible usage --- docs/source/errors.md | 3 +- .../tagOrInaccessibleUsedWithExternal.test.ts | 173 ++++++++++++++++++ .../__tests__/tagUsedWithExternal.test.ts | 89 --------- .../validate/preComposition/index.ts | 2 +- ...s => tagOrInaccessibleUsedWithExternal.ts} | 31 +++- 5 files changed, 201 insertions(+), 97 deletions(-) create mode 100644 federation-js/src/composition/validate/preComposition/__tests__/tagOrInaccessibleUsedWithExternal.test.ts delete mode 100644 federation-js/src/composition/validate/preComposition/__tests__/tagUsedWithExternal.test.ts rename federation-js/src/composition/validate/preComposition/{tagUsedWithExternal.ts => tagOrInaccessibleUsedWithExternal.ts} (56%) diff --git a/docs/source/errors.md b/docs/source/errors.md index e204e4cc3..c7f40142c 100644 --- a/docs/source/errors.md +++ b/docs/source/errors.md @@ -53,11 +53,12 @@ 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. | -## `@tag` +## 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 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/__tests__/tagUsedWithExternal.test.ts b/federation-js/src/composition/validate/preComposition/__tests__/tagUsedWithExternal.test.ts deleted file mode 100644 index 4ff4e39c3..000000000 --- a/federation-js/src/composition/validate/preComposition/__tests__/tagUsedWithExternal.test.ts +++ /dev/null @@ -1,89 +0,0 @@ -import { tagUsedWithExternal } from '..'; -import { - gql, - graphqlErrorSerializer, -} from 'apollo-federation-integration-testsuite'; - -expect.addSnapshotSerializer(graphqlErrorSerializer); - -describe('tagUsedWithExternal', () => { - 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 = tagUsedWithExternal(serviceA); - expect(errors).toHaveLength(0); - }); - - 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 = tagUsedWithExternal(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 = tagUsedWithExternal(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.", - }, - ] - `); - }); -}); diff --git a/federation-js/src/composition/validate/preComposition/index.ts b/federation-js/src/composition/validate/preComposition/index.ts index adb813bc7..6d0d1e4dc 100644 --- a/federation-js/src/composition/validate/preComposition/index.ts +++ b/federation-js/src/composition/validate/preComposition/index.ts @@ -4,4 +4,4 @@ export { keyFieldsMissingExternal } from './keyFieldsMissingExternal'; export { reservedFieldUsed } from './reservedFieldUsed'; export { duplicateEnumOrScalar } from './duplicateEnumOrScalar'; export { duplicateEnumValue } from './duplicateEnumValue'; -export { tagUsedWithExternal } from './tagUsedWithExternal'; +export { tagOrInaccessibleUsedWithExternal } from './tagOrInaccessibleUsedWithExternal'; diff --git a/federation-js/src/composition/validate/preComposition/tagUsedWithExternal.ts b/federation-js/src/composition/validate/preComposition/tagOrInaccessibleUsedWithExternal.ts similarity index 56% rename from federation-js/src/composition/validate/preComposition/tagUsedWithExternal.ts rename to federation-js/src/composition/validate/preComposition/tagOrInaccessibleUsedWithExternal.ts index 35c2ce7ed..815b39f59 100644 --- a/federation-js/src/composition/validate/preComposition/tagUsedWithExternal.ts +++ b/federation-js/src/composition/validate/preComposition/tagOrInaccessibleUsedWithExternal.ts @@ -13,10 +13,11 @@ import { } from '../../utils'; /** - * There are no fields with both @tag and @external. We're only concerned with - * the xTypeExtension nodes because @external is not allowed on base types. + * 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 tagUsedWithExternal = ({ +export const tagOrInaccessibleUsedWithExternal = ({ name: serviceName, typeDefs, }: ServiceDefinition) => { @@ -28,14 +29,17 @@ export const tagUsedWithExternal = ({ }); function fieldsVisitor( - typeDefinition: - | ObjectTypeExtensionNode - | InterfaceTypeExtensionNode, + 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) { @@ -52,6 +56,21 @@ export const tagUsedWithExternal = ({ ), ); } + + 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, + ), + ); + } } }