Skip to content

Commit

Permalink
fix: strip @external fields in interface extensions (#2848)
Browse files Browse the repository at this point in the history
Before this change, including an external field in an interface
extension would fail composition in the gateway with an error:

```
Field "Product.id" already exists in the schema. It cannot also be defined in this type extension.
```
  • Loading branch information
lennyburdette authored and JakeDawkins committed Jun 17, 2019
1 parent e1d654c commit c883224
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 26 deletions.
11 changes: 11 additions & 0 deletions packages/apollo-federation/src/composition/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ describe('Composition utility functions', () => {
type Mutation {
updateProduct: Product
}
extend interface Account @key(fields: "id") {
id: ID! @external
}
`;

const {
Expand All @@ -37,6 +41,8 @@ describe('Composition utility functions', () => {
type Mutation {
updateProduct: Product
}
extend interface Account @key(fields: "id")
`);

expect(strippedFields).toMatchInlineSnapshot(`
Expand All @@ -46,6 +52,11 @@ describe('Composition utility functions', () => {
"parentTypeName": "Product",
"serviceName": "serviceA",
},
Object {
"field": id: ID! @external,
"parentTypeName": "Account",
"serviceName": "serviceA",
},
]
`);
});
Expand Down
75 changes: 49 additions & 26 deletions packages/apollo-federation/src/composition/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'apollo-server-env';
import {
ObjectTypeDefinitionNode,
InterfaceTypeExtensionNode,
FieldDefinitionNode,
Kind,
StringValueNode,
Expand Down Expand Up @@ -45,7 +46,10 @@ export function mapFieldNamesToServiceName<Node extends { name: NameNode }>(

export function findDirectivesOnTypeOrField(
node: Maybe<
ObjectTypeDefinitionNode | ObjectTypeExtensionNode | FieldDefinitionNode
| ObjectTypeDefinitionNode
| ObjectTypeExtensionNode
| FieldDefinitionNode
| InterfaceTypeExtensionNode
>,
directiveName: string,
) {
Expand All @@ -66,36 +70,55 @@ export function stripExternalFieldsFromTypeDefs(
const strippedFields: ExternalFieldDefinition[] = [];

const typeDefsWithoutExternalFields = visit(typeDefs, {
ObjectTypeExtension(node) {
let fields = node.fields;
if (fields) {
fields = fields.filter(field => {
const externalDirectives = findDirectivesOnTypeOrField(
field,
'external',
);

if (externalDirectives.length > 0) {
strippedFields.push({
field,
parentTypeName: node.name.value,
serviceName,
});
return false;
}
return true;
});
}
return {
...node,
fields,
};
},
ObjectTypeExtension: removeExternalFieldsFromExtensionVisitor(
strippedFields,
serviceName,
),
InterfaceTypeExtension: removeExternalFieldsFromExtensionVisitor(
strippedFields,
serviceName,
),
}) as DocumentNode;

return { typeDefsWithoutExternalFields, strippedFields };
}

/**
* Returns a closure that strips fields marked with `@external` and adds them
* to an array.
* @param collector
* @param serviceName
*/
function removeExternalFieldsFromExtensionVisitor<
T extends InterfaceTypeExtensionNode | ObjectTypeExtensionNode
>(collector: ExternalFieldDefinition[], serviceName: string) {
return (node: T) => {
let fields = node.fields;
if (fields) {
fields = fields.filter(field => {
const externalDirectives = findDirectivesOnTypeOrField(
field,
'external',
);

if (externalDirectives.length > 0) {
collector.push({
field,
parentTypeName: node.name.value,
serviceName,
});
return false;
}
return true;
});
}
return {
...node,
fields,
};
};
}

export function parseSelections(source: string) {
return (parse(`query { ${source} }`)
.definitions[0] as OperationDefinitionNode).selectionSet.selections;
Expand Down

0 comments on commit c883224

Please sign in to comment.