Skip to content

Commit

Permalink
fix(gateway): merge selection sets of @requires fields (apollograph…
Browse files Browse the repository at this point in the history
…ql/apollo-server#4064)

Previously, it was possible for the gateway to build and issue queries
for resolving fields that resembled the following:

```graphql
{
  product { id }
  product { price }
}
```

In the above example, the second selection set will take precedence and
the first ignored, meaning the query _ought_ to be:

```graphql
{
  product { id price }
}
```

This is accomplished in this PR by merging the selection sets recursively.

fixes apollographql/apollo-server#3848

Co-authored-by: Lenny Burdette <lennyburdette@gmail.com>
Apollo-Orig-Commit-AS: apollographql/apollo-server@3cdcd06
  • Loading branch information
trevor-scheer and lennyburdette committed May 21, 2020
2 parents 8bb177d + b90764a commit 6dc2576
Show file tree
Hide file tree
Showing 5 changed files with 290 additions and 22 deletions.
3 changes: 2 additions & 1 deletion gateway-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +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 appropriate changes within that release will be moved into the new section.
- _Nothing yet! Stay tuned._
- __FIX__: Collapse nested required fields into a single body in the query plan. Before, some nested fields' selection sets were getting split, causing some of their subfields to be dropped when executing the query. This fix collapses the split selection sets into one. [#4064](https://github.com/apollographql/apollo-server/pull/4064)


## 0.16.0

Expand Down
54 changes: 34 additions & 20 deletions gateway-js/src/FieldSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
GraphQLObjectType,
} from 'graphql';
import { getResponseName } from './utilities/graphql';
import { partition, groupBy } from './utilities/array';

export interface Field<
TParent extends GraphQLCompositeType = GraphQLCompositeType
Expand Down Expand Up @@ -45,25 +46,6 @@ export function matchesField(field: Field) {
};
}

function groupBy<T, U>(keyFunction: (element: T) => U) {
return (iterable: Iterable<T>) => {
const result = new Map<U, T[]>();

for (const element of iterable) {
const key = keyFunction(element);
const group = result.get(key);

if (group) {
group.push(element);
} else {
result.set(key, [element]);
}
}

return result;
};
}

export const groupByResponseName = groupBy<Field, string>(field =>
getResponseName(field.fieldNode)
);
Expand Down Expand Up @@ -147,6 +129,38 @@ function mergeSelectionSets(fieldNodes: FieldNode[]): SelectionSetNode {

return {
kind: 'SelectionSet',
selections,
selections: mergeFieldNodeSelectionSets(selections),
};
}

function mergeFieldNodeSelectionSets(
selectionNodes: SelectionNode[],
): SelectionNode[] {
const [fieldNodes, fragmentNodes] = partition(
selectionNodes,
(node): node is FieldNode => node.kind === Kind.FIELD,
);

const [aliasedFieldNodes, nonAliasedFieldNodes] = partition(
fieldNodes,
node => !!node.alias,
);

const mergedFieldNodes = Array.from(
groupBy((node: FieldNode) => node.name.value)(
nonAliasedFieldNodes,
).values(),
).map((nodesWithSameName) => {
const node = nodesWithSameName[0];
if (node.selectionSet) {
node.selectionSet.selections = mergeFieldNodeSelectionSets(
nodesWithSameName.flatMap(
(node) => node.selectionSet?.selections || [],
),
);
}
return node;
});

return [...mergedFieldNodes, ...aliasedFieldNodes, ...fragmentNodes];
}
1 change: 0 additions & 1 deletion gateway-js/src/__tests__/integration/complex-key.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ it('works fetches data correctly with complex / nested @key fields', async () =>
organization {
id
__typename
id
}
}
}
Expand Down
227 changes: 227 additions & 0 deletions gateway-js/src/__tests__/integration/requires.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import gql from 'graphql-tag';
import { execute } from '../execution-utils';
import { serializeQueryPlan } from '../..';

it('supports passing additional fields defined by a requires', async () => {
const query = `#graphql
Expand Down Expand Up @@ -38,3 +40,228 @@ it('supports passing additional fields defined by a requires', async () => {
expect(queryPlan).toCallService('product');
expect(queryPlan).toCallService('books');
});

const serviceA = {
name: 'a',
typeDefs: gql`
type Query {
user: User
}
type User @key(fields: "id") {
id: ID!
preferences: Preferences
}
type Preferences {
favorites: Things
}
type Things {
color: String
animal: String
}
`,
resolvers: {
Query: {
user() {
return {
id: '1',
preferences: {
favorites: { color: 'limegreen', animal: 'platypus' },
},
};
},
},
},
};

const serviceB = {
name: 'b',
typeDefs: gql`
extend type User @key(fields: "id") {
id: ID! @external
preferences: Preferences @external
favoriteColor: String
@requires(fields: "preferences { favorites { color } }")
favoriteAnimal: String
@requires(fields: "preferences { favorites { animal } }")
}
extend type Preferences {
favorites: Things @external
}
extend type Things {
color: String @external
animal: String @external
}
`,
resolvers: {
User: {
favoriteColor(user: any) {
return user.preferences.favorites.color;
},
favoriteAnimal(user: any) {
return user.preferences.favorites.animal;
},
},
},
};

it('collapses nested requires', async () => {
const query = `#graphql
query UserFavorites {
user {
favoriteColor
favoriteAnimal
}
}
`;

const { data, errors, queryPlan } = await execute(
{
query,
},
[serviceA, serviceB],
);

expect(errors).toEqual(undefined);

expect(serializeQueryPlan(queryPlan)).toMatchInlineSnapshot(`
"QueryPlan {
Sequence {
Fetch(service: \\"a\\") {
{
user {
__typename
id
preferences {
favorites {
color
animal
}
}
}
}
},
Flatten(path: \\"user\\") {
Fetch(service: \\"b\\") {
{
... on User {
__typename
id
preferences {
favorites {
color
animal
}
}
}
} =>
{
... on User {
favoriteColor
favoriteAnimal
}
}
},
},
},
}"
`);

expect(data).toEqual({
user: {
favoriteAnimal: 'platypus',
favoriteColor: 'limegreen',
},
});

expect(queryPlan).toCallService('a');
expect(queryPlan).toCallService('b');
});

it('collapses nested requires with user-defined fragments', async () => {
const query = `#graphql
query UserFavorites {
user {
favoriteAnimal
...favoriteColor
}
}
fragment favoriteColor on User {
preferences {
favorites {
color
}
}
}
`;

const { data, errors, queryPlan } = await execute(
{
query,
},
[serviceA, serviceB],
);

expect(errors).toEqual(undefined);

expect(serializeQueryPlan(queryPlan)).toMatchInlineSnapshot(`
"QueryPlan {
Sequence {
Fetch(service: \\"a\\") {
{
user {
__typename
id
preferences {
favorites {
animal
color
}
}
}
}
},
Flatten(path: \\"user\\") {
Fetch(service: \\"b\\") {
{
... on User {
__typename
id
preferences {
favorites {
animal
color
}
}
}
} =>
{
... on User {
favoriteAnimal
}
}
},
},
},
}"
`);

expect(data).toEqual({
user: {
favoriteAnimal: 'platypus',
preferences: {
favorites: {
color: 'limegreen',
},
},
},
});

expect(queryPlan).toCallService('a');
expect(queryPlan).toCallService('b');
});
27 changes: 27 additions & 0 deletions gateway-js/src/utilities/array.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ export function compactMap<T, U>(
);
}

export function partition<T, U extends T>(
array: T[],
predicate: (element: T, index: number, array: T[]) => element is U,
): [U[], T[]];
export function partition<T>(
array: T[],
predicate: (element: T, index: number, array: T[]) => boolean,
): [T[], T[]];
export function partition<T>(
array: T[],
predicate: (element: T, index: number, array: T[]) => boolean,
Expand Down Expand Up @@ -48,3 +56,22 @@ export function findAndExtract<T>(

return [array[index], remaining];
}

export function groupBy<T, U>(keyFunction: (element: T) => U) {
return (iterable: Iterable<T>) => {
const result = new Map<U, T[]>();

for (const element of iterable) {
const key = keyFunction(element);
const group = result.get(key);

if (group) {
group.push(element);
} else {
result.set(key, [element]);
}
}

return result;
};
}

0 comments on commit 6dc2576

Please sign in to comment.