Skip to content

Commit

Permalink
fix(gateway): Handle @external validation edge case for interface i…
Browse files Browse the repository at this point in the history
…mplementors (apollographql/apollo-server#4284)

One edge case on the validation of externals being used is when concrete
types need to be defined in multiple services for an interface to be
returned as a type for a field. To return an interface, you need to
defined its possible implementations to let the schema know what all it
could support but you also need to mark its fields as external (at least
the ones you can resolve) so that you don't try and take over ownership
of an entities fields. The current implementation doesn't take this into
account so you fail validation and can't compose the graph.

Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>
Apollo-Orig-Commit-AS: apollographql/apollo-server@abe72d4
  • Loading branch information
James Baxley and trevor-scheer authored Jun 19, 2020
1 parent bb95ebf commit 3ea6b00
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 1 deletion.
2 changes: 1 addition & 1 deletion federation-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
},
]
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> = 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',
Expand Down

0 comments on commit 3ea6b00

Please sign in to comment.