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.

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

Fix fragment reuse in subgraph fetches #1911

Merged
merged 2 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions gateway-js/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The Federation v0.x equivalent for this package can be found [here](https://github.com/apollographql/federation/blob/version-0.x/gateway-js/CHANGELOG.md) on the `version-0.x` branch of this repo.

- Fix fragment reuse in subgraph fetches [PR #1911](https://github.com/apollographql/federation/pull/1911).

## 2.1.0-alpha.1

- Expose document representation of sub-query request within GraphQLDataSourceProcessOptions so that it is available to RemoteGraphQLDataSource.process and RemoteGraphQLDataSource.willSendRequest [PR#1878](https://github.com/apollographql/federation/pull/1878)
Expand Down
8 changes: 6 additions & 2 deletions gateway-js/src/__tests__/buildQueryPlan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -771,13 +771,17 @@ describe('buildQueryPlan', () => {
topProducts {
__typename
... on Book {
price
...Price
}
... on Furniture {
price
...Price
}
}
}

fragment Price on Product {
price
}
},
}
`);
Expand Down
14 changes: 8 additions & 6 deletions gateway-js/src/__tests__/integration/abstract-types.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1626,14 +1626,16 @@ it('when exploding types through multiple levels', async () => {
} =>
{
... on Book {
reviews {
body
}
...ProductReviews
}
... on Furniture {
reviews {
body
}
...ProductReviews
}
}

fragment ProductReviews on Product {
reviews {
body
}
}
},
Expand Down
4 changes: 3 additions & 1 deletion gateway-js/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { HeadersInit } from 'node-fetch';
import { GraphQLRequestContextExecutionDidStart } from 'apollo-server-types';
import type { Logger } from '@apollo/utils.logger';
import { GraphQLDataSource } from './datasources/types';
import { QueryPlan } from '@apollo/query-planner';
import { QueryPlan, QueryPlannerConfig } from '@apollo/query-planner';
import { OperationContext } from './operationContext';
import { ServiceMap } from './executeQueryPlan';
import { ServiceDefinition } from "@apollo/federation-internals";
Expand Down Expand Up @@ -129,6 +129,8 @@ interface GatewayConfigBase {
experimental_autoFragmentization?: boolean;
fetcher?: Fetcher;
serviceHealthCheck?: boolean;

queryPlannerConfig?: QueryPlannerConfig;
}

// TODO(trevor:removeServiceList)
Expand Down
2 changes: 1 addition & 1 deletion gateway-js/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ export class ApolloGateway implements GraphQLService {
this.schema = addExtensions(
wrapSchemaWithAliasResolver(this.apiSchema.toGraphQLJSSchema()),
);
this.queryPlanner = new QueryPlanner(coreSchema);
this.queryPlanner = new QueryPlanner(coreSchema, this.config.queryPlannerConfig);

// Notify onSchemaChange listeners of the updated schema
if (!legacyDontNotifyOnSchemaChangeListeners) {
Expand Down
280 changes: 181 additions & 99 deletions internals-js/src/__tests__/operations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,133 +13,215 @@ function parseSchema(schema: string): Schema {
}
}

test('fragments optimization of selection sets', () => {
const schema = parseSchema(`
type Query {
t: T1
}

interface I {
b: Int
}
describe('fragments optimization', () => {
test('handles fragments using other fragments', () => {
const schema = parseSchema(`
type Query {
t: T1
}

type T1 {
a: Int
b: Int
u: U
}
interface I {
b: Int
}

type T2 {
x: String
y: String
b: Int
u: U
}
type T1 {
a: Int
b: Int
u: U
}

union U = T1 | T2
`);
type T2 {
x: String
y: String
b: Int
u: U
}

const operation = parseOperation(schema, `
fragment OnT1 on T1 {
a
b
}
union U = T1 | T2
`);

fragment OnT2 on T2 {
x
y
}
const operation = parseOperation(schema, `
fragment OnT1 on T1 {
a
b
}

fragment OnI on I {
b
}
fragment OnT2 on T2 {
x
y
}

fragment OnU on U {
...OnI
...OnT1
...OnT2
}
fragment OnI on I {
b
}

query {
t {
fragment OnU on U {
...OnI
...OnT1
...OnT2
...OnI
u {
...OnU
}

query {
t {
...OnT1
...OnT2
...OnI
u {
...OnU
}
}
}
}
`);
`);

const withoutFragments = parseOperation(schema, operation.toString(true, true));
expect(withoutFragments.toString()).toMatchString(`
{
t {
... on T1 {
a
b
}
... on T2 {
x
y
}
... on I {
b
}
u {
... on U {
... on I {
b
}
... on T1 {
a
b
}
... on T2 {
x
y
}
}
}
}
}
`);

const optimized = withoutFragments.optimize(operation.selectionSet.fragments!);
// Note that we expect onU to *not* be recreated because, by default, optimize only
// add add back a fragment if it is used at least twice (otherwise, the fragment just
// make the query bigger).
expect(optimized.toString()).toMatchString(`
fragment OnT1 on T1 {
a
b
}

const withoutFragments = parseOperation(schema, operation.toString(true, true));
expect(withoutFragments.toString()).toMatchString(`
{
t {
... on T1 {
a
b
fragment OnT2 on T2 {
x
y
}

fragment OnI on I {
b
}

{
t {
...OnT1
...OnT2
...OnI
u {
...OnI
...OnT1
...OnT2
}
}
... on T2 {
}
`);
});

test('handles fragments with nested selections', () => {
const schema = parseSchema(`
type Query {
t1a: T1
t2a: T1
}

type T1 {
t2: T2
}

type T2 {
x: String
y: String
}
`);

const operation = parseOperation(schema, `
fragment OnT1 on T1 {
t2 {
x
y
}
... on I {
b
}

query {
t1a {
...OnT1
t2 {
y
}
}
u {
... on U {
... on I {
b
}
... on T1 {
a
b
t2a {
...OnT1
}
}
`);

const withoutFragments = parseOperation(schema, operation.toString(true, true));
expect(withoutFragments.toString()).toMatchString(`
{
t1a {
... on T1 {
t2 {
x
}
... on T2 {
}
t2 {
y
}
}
t2a {
... on T1 {
t2 {
x
y
}
}
}
}
}
`);
`);

const optimized = withoutFragments.optimize(operation.selectionSet.fragments!);
// Note that we expect onU to *not* be recreated because, by default, optimize only
// add add back a fragment if it is used at least twice (otherwise, the fragment just
// make the query bigger).
expect(optimized.toString()).toMatchString(`
fragment OnT1 on T1 {
a
b
}

fragment OnT2 on T2 {
x
y
}

fragment OnI on I {
b
}
const optimized = withoutFragments.optimize(operation.selectionSet.fragments!);
expect(optimized.toString()).toMatchString(`
fragment OnT1 on T1 {
t2 {
x
}
}

{
t {
...OnT1
...OnT2
...OnI
u {
... on U {
...OnI
...OnT1
...OnT2
{
t1a {
...OnT1
t2 {
y
}
}
t2a {
...OnT1
}
}
}
`);
`);
});
});

describe('selection set freezing', () => {
Expand Down
Loading