Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix potential QP issue with shareable root fields #2239

Merged
merged 2 commits into from
Nov 10, 2022
Merged

Fix potential QP issue with shareable root fields #2239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d172f69
Fix potential QP issue with shareable root fields
Nov 8, 2022
90c9d8c
changelogs
Nov 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions gateway-js/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The

## vNext

- Fix potential QP issue with shareable root fields [PR #2239](https://github.com/apollographql/federation/pull/2239).
- Correctly reject field names starting with `__` [PR #2237](https://github.com/apollographql/federation/pull/2237).
- Fix error when a skipped enum value had directives applied [PR #2232](https://github.com/apollographql/federation/pull/2232).
- Preserve default values of input object fields [PR #2218](https://github.com/apollographql/federation/pull/2218).
Expand Down
1 change: 1 addition & 0 deletions query-planner-js/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The

## vNext

- Fix potential QP issue with shareable root fields [PR #2239](https://github.com/apollographql/federation/pull/2239).

## 2.1.4

Expand Down
243 changes: 190 additions & 53 deletions query-planner-js/src/__tests__/buildPlan.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,69 +24,154 @@ export function composeAndCreatePlannerWithOptions(services: ServiceDefinition[]
];
}

test('can use same root operation from multiple subgraphs in parallel', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
me: User! @shareable
}
describe('shareable root fields', () => {
test('can use same root operation from multiple subgraphs in parallel', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
me: User! @shareable
}

type User @key(fields: "id") {
id: ID!
prop1: String
}
`
}
type User @key(fields: "id") {
id: ID!
prop1: String
}
`
}

const subgraph2 = {
name: 'Subgraph2',
typeDefs: gql`
type Query {
me: User! @shareable
}
const subgraph2 = {
name: 'Subgraph2',
typeDefs: gql`
type Query {
me: User! @shareable
}

type User @key(fields: "id") {
id: ID!
prop2: String
}
`
}
type User @key(fields: "id") {
id: ID!
prop2: String
}
`
}

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
const operation = operationFromDocument(api, gql`
{
me {
prop1
prop2
const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
const operation = operationFromDocument(api, gql`
{
me {
prop1
prop2
}
}
}
`);
`);

const plan = queryPlanner.buildQueryPlan(operation);
// Note that even though we have keys, it is faster to query both
// subgraphs in parallel for each property than querying one first
// and then using the key.
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Parallel {
Fetch(service: "Subgraph1") {
{
me {
prop1
const plan = queryPlanner.buildQueryPlan(operation);
// Note that even though we have keys, it is faster to query both
// subgraphs in parallel for each property than querying one first
// and then using the key.
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Parallel {
Fetch(service: "Subgraph1") {
{
me {
prop1
}
}
}
},
Fetch(service: "Subgraph2") {
{
me {
prop2
},
Fetch(service: "Subgraph2") {
{
me {
prop2
}
}
}
},
},
},
}
`);
});

test('handles root operation shareable in many subgraphs', () => {
const fieldCount = 4;
const fields = [...Array(fieldCount).keys()].map((i) => `f${i}`);
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type User @key(fields: "id") {
id: ID!
${fields.map((f) => `${f}: Int\n`)}
}
`
}
`);

const subgraph2 = {
name: 'Subgraph2',
typeDefs: gql`
type Query {
me: User! @shareable
}

type User @key(fields: "id") {
id: ID!
}
`
}

const subgraph3 = {
name: 'Subgraph3',
typeDefs: gql`
type Query {
me: User! @shareable
}

type User @key(fields: "id") {
id: ID!
}
`
}

const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2, subgraph3);
const operation = operationFromDocument(api, gql`
{
me {
${fields.map((f) => `${f}\n`)}
}
}
`);

const plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Sequence {
Fetch(service: "Subgraph2") {
{
me {
__typename
id
}
}
},
Flatten(path: "me") {
Fetch(service: "Subgraph1") {
{
... on User {
__typename
id
}
} =>
{
... on User {
f0
f1
f2
f3
}
}
},
},
},
}
`);
});
});

test('pick keys that minimize fetches', () => {
Expand Down Expand Up @@ -4285,3 +4370,55 @@ describe('__typename handling', () => {
`);
});
});

describe('mutations', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewer: that test is not really directly related to this patch, but I notice that buildPlan.test.ts had no simple Mutation test. Mutation were still tested elsewhere (in the plan execution tests mostly, but also in plenty of older test files) but as I often use that one test file as a shortcut for "have I broken query planning", it's convenient to have at least a simple mutation test there.

it('executes mutation operations in sequence', () => {
const subgraph1 = {
name: 'Subgraph1',
typeDefs: gql`
type Query {
q1: Int
}

type Mutation {
m1: Int
}
`
}

const subgraph2 = {
name: 'Subgraph2',
typeDefs: gql`
type Mutation {
m2: Int
}
`
}

let [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2);
let operation = operationFromDocument(api, gql`
mutation {
m2
m1
}
`);

let plan = queryPlanner.buildQueryPlan(operation);
expect(plan).toMatchInlineSnapshot(`
QueryPlan {
Sequence {
Fetch(service: "Subgraph2") {
{
m2
}
},
Fetch(service: "Subgraph1") {
{
m1
}
},
},
}
`);
});
});
Loading