diff --git a/federation-js/CHANGELOG.md b/federation-js/CHANGELOG.md index dcaf81e89..d3b9dcab5 100644 --- a/federation-js/CHANGELOG.md +++ b/federation-js/CHANGELOG.md @@ -4,7 +4,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. -- _Nothing yet! Stay tuned._ +- Handle `@external` validation edge case for interface implementors [#4284](https://github.com/apollographql/apollo-server/pull/4284) ## 0.16.7 diff --git a/federation-js/src/composition/validate/postComposition/__tests__/externalUnused.test.ts b/federation-js/src/composition/validate/postComposition/__tests__/externalUnused.test.ts index ac768b07d..e3156c017 100644 --- a/federation-js/src/composition/validate/postComposition/__tests__/externalUnused.test.ts +++ b/federation-js/src/composition/validate/postComposition/__tests__/externalUnused.test.ts @@ -302,4 +302,79 @@ describe('externalUnused', () => { const warnings = validateExternalUnused({ schema, serviceList }); expect(warnings).toEqual([]); }); + + it('does not error when @external is used on a field of a concrete type that implements a shared field of an implemented interface', () => { + const serviceA = { + typeDefs: gql` + type Car implements Vehicle @key(fields: "id") { + id: ID! + speed: Int + } + interface Vehicle { + id: ID! + speed: Int + } + `, + name: 'serviceA', + }; + const serviceB = { + typeDefs: gql` + extend type Car implements Vehicle @key(fields: "id") { + id: ID! @external + speed: Int @external + } + interface Vehicle { + id: ID! + speed: Int + } + `, + name: 'serviceB', + }; + const serviceList = [serviceA, serviceB]; + const { schema } = composeServices(serviceList); + const errors = validateExternalUnused({ schema, serviceList }); + expect(errors).toHaveLength(0); + }); + + it('does error when @external is used on a field of a concrete type is not shared by its implemented interface', () => { + const serviceA = { + typeDefs: gql` + type Car implements Vehicle @key(fields: "id") { + id: ID! + speed: Int + wheelSize: Int + } + interface Vehicle { + id: ID! + speed: Int + } + `, + name: 'serviceA', + }; + const serviceB = { + typeDefs: gql` + extend type Car implements Vehicle @key(fields: "id") { + id: ID! @external + speed: Int @external + wheelSize: Int @external + } + interface Vehicle { + id: ID! + speed: Int + } + `, + name: 'serviceB', + }; + const serviceList = [serviceA, serviceB]; + const { schema } = composeServices(serviceList); + const errors = validateExternalUnused({ schema, serviceList }); + expect(errors).toMatchInlineSnapshot(` + Array [ + Object { + "code": "EXTERNAL_UNUSED", + "message": "[serviceB] Car.wheelSize -> is marked as @external but is not used by a @requires, @key, or @provides directive.", + }, + ] + `); + }); }); diff --git a/federation-js/src/composition/validate/postComposition/externalUnused.ts b/federation-js/src/composition/validate/postComposition/externalUnused.ts index 3da582ec6..1cdeddede 100644 --- a/federation-js/src/composition/validate/postComposition/externalUnused.ts +++ b/federation-js/src/composition/validate/postComposition/externalUnused.ts @@ -160,6 +160,55 @@ export const externalUnused: PostCompositionValidator = ({ schema }) => { if (hasMatchingRequiresOnType) continue; + /** + * @external fields can be required when an interface is returned by + * a field and its concrete implementations need to be defined in a + * service which use non-key fields from other services. Take for example: + * + * // Service A + * type Car implements Vehicle @key(fields: "id") { + * id: ID! + * speed: Int + * } + * + * interface Vehicle { + * id: ID! + * speed: Int + * } + * + * // Service B + * type Query { + * vehicles: [Vehicle] + * } + * + * extend type Car implements Vehicle @key(fields: "id") { + * id: ID! @external + * speed: Int @external + * } + * + * interface Vehicle { + * id: ID! + * speed: Int + * } + * + * Service B defines Car.speed as an external field which is okay + * because it is required for Query.vehicles to exist in the schema + */ + const fieldsOnInterfacesImplementedByParentType: Set = new Set(); + + // Loop over the parent's interfaces + for (const _interface of parentType.getInterfaces()) { + // Collect the field names from each interface in a set + for (const fieldName in _interface.getFields()) { + fieldsOnInterfacesImplementedByParentType.add(fieldName); + } + } + + // If the set contains our field's name, no error is generated + if (fieldsOnInterfacesImplementedByParentType.has(externalFieldName)) { + continue; + } + errors.push( errorWithCode( 'EXTERNAL_UNUSED',