From c98704dd338a97033ec6b1a439752261b7a5e87a Mon Sep 17 00:00:00 2001 From: jbaxleyiii Date: Thu, 5 Dec 2019 21:09:41 -0500 Subject: [PATCH 1/2] This commit is the very first stage (of many to come) to address the current problematic way federation handles abstract types. Prior to this commit, any query that asked for a field on an interface would be expanded to a selection of type conditions for all possible types. This would result in extremely large operations sent to underlying services. This commit creates a first shortcut of this process when we know that every possible type of an interface are all contained within a single service. This lets us prevent explosion of types that can be handled by an invidual service. Co-authored-by: Delyan Ruskov --- packages/apollo-gateway/CHANGELOG.md | 1 + .../integration/abstract-types.test.ts | 547 ++++++++++++++++++ packages/apollo-gateway/src/buildQueryPlan.ts | 33 +- 3 files changed, 580 insertions(+), 1 deletion(-) diff --git a/packages/apollo-gateway/CHANGELOG.md b/packages/apollo-gateway/CHANGELOG.md index 6581adf68c1..708b8c76c59 100644 --- a/packages/apollo-gateway/CHANGELOG.md +++ b/packages/apollo-gateway/CHANGELOG.md @@ -5,6 +5,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 the appropriate changes within that release will be moved into the new section. * Fix onSchemaChange callbacks for unmanaged configs [#3605](https://github.com/apollographql/apollo-server/pull/3605) +* Reduce interface expansion for types contained to a single service [#3582](https://github.com/apollographql/apollo-server/pull/3582) # v0.11.4 diff --git a/packages/apollo-gateway/src/__tests__/integration/abstract-types.test.ts b/packages/apollo-gateway/src/__tests__/integration/abstract-types.test.ts index 0870dce5453..78c012f8e08 100644 --- a/packages/apollo-gateway/src/__tests__/integration/abstract-types.test.ts +++ b/packages/apollo-gateway/src/__tests__/integration/abstract-types.test.ts @@ -6,6 +6,11 @@ import * as inventory from '../__fixtures__/schemas/inventory'; import * as product from '../__fixtures__/schemas/product'; import * as reviews from '../__fixtures__/schemas/reviews'; +import { astSerializer, queryPlanSerializer } from '../../snapshotSerializers'; + +expect.addSnapshotSerializer(astSerializer); +expect.addSnapshotSerializer(queryPlanSerializer); + it('handles an abstract type from the base service', async () => { const query = gql` query GetProduct($upc: String!) { @@ -35,6 +40,65 @@ it('handles an abstract type from the base service', async () => { }); expect(queryPlan).toCallService('product'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "product") { + { + product(upc: $upc) { + __typename + ... on Book { + upc + __typename + isbn + price + } + ... on Furniture { + upc + name + price + } + } + } + }, + Flatten(path: "product") { + Fetch(service: "books") { + { + ... on Book { + __typename + isbn + } + } => + { + ... on Book { + __typename + isbn + title + year + } + } + }, + }, + Flatten(path: "product") { + Fetch(service: "product") { + { + ... on Book { + __typename + isbn + title + year + } + } => + { + ... on Book { + name + } + } + }, + }, + }, + } + `); }); it('can request fields on extended interfaces', async () => { @@ -59,6 +123,49 @@ it('can request fields on extended interfaces', async () => { expect(data).toEqual({ product: { inStock: true } }); expect(queryPlan).toCallService('product'); expect(queryPlan).toCallService('inventory'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "product") { + { + product(upc: $upc) { + __typename + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + sku + } + } + } + }, + Flatten(path: "product") { + Fetch(service: "inventory") { + { + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + sku + } + } => + { + ... on Book { + inStock + } + ... on Furniture { + inStock + } + } + }, + }, + }, + } + `); }); it('can request fields on extended types that implement an interface', async () => { @@ -85,6 +192,50 @@ it('can request fields on extended types that implement an interface', async () expect(data).toEqual({ product: { inStock: true, isHeavy: false } }); expect(queryPlan).toCallService('product'); expect(queryPlan).toCallService('inventory'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "product") { + { + product(upc: $upc) { + __typename + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + sku + } + } + } + }, + Flatten(path: "product") { + Fetch(service: "inventory") { + { + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + sku + } + } => + { + ... on Book { + inStock + } + ... on Furniture { + inStock + isHeavy + } + } + }, + }, + }, + } + `); }); it('prunes unfilled type conditions', async () => { @@ -114,6 +265,51 @@ it('prunes unfilled type conditions', async () => { expect(data).toEqual({ product: { inStock: true, isHeavy: false } }); expect(queryPlan).toCallService('product'); expect(queryPlan).toCallService('inventory'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "product") { + { + product(upc: $upc) { + __typename + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + sku + } + } + } + }, + Flatten(path: "product") { + Fetch(service: "inventory") { + { + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + sku + } + } => + { + ... on Book { + inStock + isCheckedOut + } + ... on Furniture { + inStock + isHeavy + } + } + }, + }, + }, + } + `); }); it('fetches interfaces returned from other services', async () => { @@ -152,6 +348,86 @@ it('fetches interfaces returned from other services', async () => { expect(queryPlan).toCallService('accounts'); expect(queryPlan).toCallService('reviews'); expect(queryPlan).toCallService('product'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "accounts") { + { + me { + __typename + id + } + } + }, + Flatten(path: "me") { + Fetch(service: "reviews") { + { + ... on User { + __typename + id + } + } => + { + ... on User { + reviews { + product { + __typename + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } + } + } + } + }, + }, + Parallel { + Flatten(path: "me.reviews.@.product") { + Fetch(service: "product") { + { + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } => + { + ... on Book { + price + } + ... on Furniture { + price + } + } + }, + }, + Flatten(path: "me.reviews.@.product") { + Fetch(service: "books") { + { + ... on Book { + __typename + isbn + } + } => + { + ... on Book { + title + } + } + }, + }, + }, + }, + } + `); }); it('fetches composite fields from a foreign type casted to an interface [@provides field]', async () => { @@ -190,6 +466,108 @@ it('fetches composite fields from a foreign type casted to an interface [@provid expect(queryPlan).toCallService('accounts'); expect(queryPlan).toCallService('reviews'); expect(queryPlan).toCallService('product'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "accounts") { + { + me { + __typename + id + } + } + }, + Flatten(path: "me") { + Fetch(service: "reviews") { + { + ... on User { + __typename + id + } + } => + { + ... on User { + reviews { + product { + __typename + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } + } + } + } + }, + }, + Parallel { + Flatten(path: "me.reviews.@.product") { + Fetch(service: "product") { + { + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } => + { + ... on Book { + price + } + ... on Furniture { + price + } + } + }, + }, + Sequence { + Flatten(path: "me.reviews.@.product") { + Fetch(service: "books") { + { + ... on Book { + __typename + isbn + } + } => + { + ... on Book { + __typename + isbn + title + year + } + } + }, + }, + Flatten(path: "me.reviews.@.product") { + Fetch(service: "product") { + { + ... on Book { + __typename + isbn + title + year + } + } => + { + ... on Book { + name + } + } + }, + }, + }, + }, + }, + } + `); }); it('allows for extending an interface from another service with fields', async () => { @@ -220,6 +598,53 @@ it('allows for extending an interface from another service with fields', async ( expect(queryPlan).toCallService('reviews'); expect(queryPlan).toCallService('product'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "product") { + { + product(upc: $upc) { + __typename + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } + } + }, + Flatten(path: "product") { + Fetch(service: "reviews") { + { + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } => + { + ... on Book { + reviews { + body + } + } + ... on Furniture { + reviews { + body + } + } + } + }, + }, + }, + } + `); }); describe('unions', () => { @@ -271,6 +696,128 @@ describe('unions', () => { expect(queryPlan).toCallService('accounts'); expect(queryPlan).toCallService('reviews'); expect(queryPlan).toCallService('product'); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "accounts") { + { + me { + __typename + id + } + } + }, + Flatten(path: "me") { + Fetch(service: "reviews") { + { + ... on User { + __typename + id + } + } => + { + ... on User { + reviews { + product { + __typename + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } + } + } + } + }, + }, + Flatten(path: "me.reviews.@.product") { + Fetch(service: "product") { + { + ... on Book { + __typename + isbn + } + ... on Furniture { + __typename + upc + } + } => + { + ... on Book { + price + } + ... on Furniture { + price + brand { + __typename + ... on Ikea { + asile + } + ... on Amazon { + referrer + } + } + } + } + }, + }, + }, + } + `); + }); + + it("doesn't expand interfaces with inline type conditions if all possibilities are fufilled by one service", async () => { + const query = gql` + query GetProducts { + topProducts { + name + } + } + `; + + const { queryPlan, errors } = await execute( + [ + { + name: 'products', + typeDefs: gql` + extend type Query { + topProducts: [Product] + } + + interface Product { + name: String + } + + type Shoe implements Product { + name: String + } + + type Car implements Product { + name: String + } + `, + }, + ], + { query }, + ); + + expect(errors).toBeUndefined(); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Fetch(service: "products") { + { + topProducts { + __typename + name + } + } + }, + } + `); }); // FIXME: turn back on when extending unions is supported in composition diff --git a/packages/apollo-gateway/src/buildQueryPlan.ts b/packages/apollo-gateway/src/buildQueryPlan.ts index 1b255cbd95c..1dcfe048e92 100644 --- a/packages/apollo-gateway/src/buildQueryPlan.ts +++ b/packages/apollo-gateway/src/buildQueryPlan.ts @@ -385,6 +385,10 @@ function splitFields( const field = fieldsForParentType[0]; const { scope, fieldDef } = field; + // If the length of possibleTypes is zero, we're nested inside a type condition + // that's impossible to fulfill and can be excluded from the query plan altogether. + if (scope.possibleTypes.length === 0) continue; + // We skip `__typename` for root types. if (fieldDef.name === TypeNameMetaFieldDef.name) { const { schema } = context; @@ -419,6 +423,33 @@ function splitFields( } else { // For interfaces however, we need to look at all possible runtime types. + /** + * The following is an optimization to prevent an explosion of type + * conditions to services when it isn't needed. If all possible runtime + * types can be fufilled by only one service then we don't need to + * expand the fields into unique type conditions. + */ + + // Collect all of the field defs on the possible runtime types + const possibleFieldDefs = scope.possibleTypes.map( + runtimeType => context.getFieldDef(runtimeType, field.fieldNode), + ); + + // If none of the field defs have a federation property, this interface's + // implementors can all be resolved within the same service. + const hasNoExtendingFieldDefs = possibleFieldDefs.every( + def => !def.federation + ); + + // With no extending field definitions, we can engage the optimization + if (hasNoExtendingFieldDefs) { + const group = groupForField(field as Field); + group.fields.push( + completeField(context, scope, group, path, fieldsForResponseName) + ); + continue; + } + // We keep track of which possible runtime parent types can be fetched // from which group, const groupsByRuntimeParentTypes = new MultiMap< @@ -475,7 +506,7 @@ function splitFields( function completeField( context: QueryPlanningContext, - scope: Scope, + scope: Scope, parentGroup: FetchGroup, path: ResponsePath, fields: FieldSet, From 19250dc5cdbc052b66c2624ad42a5ff575ff0ca3 Mon Sep 17 00:00:00 2001 From: Delyan Ruskov Date: Thu, 16 Jan 2020 13:18:09 +0000 Subject: [PATCH 2/2] Add tests to ensure gateway resolves abstract types correctly --- .../__tests__/__fixtures__/schemas/product.ts | 57 ++++++++- .../__tests__/__fixtures__/schemas/reviews.ts | 17 ++- .../src/__tests__/executeQueryPlan.test.ts | 117 ++++++++++++++++++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/product.ts b/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/product.ts index 87d5bae1700..e1de28eb495 100644 --- a/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/product.ts +++ b/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/product.ts @@ -8,6 +8,7 @@ export const typeDefs = gql` extend type Query { product(upc: String!): Product + vehicle(id: String!): Vehicle topProducts(first: Int = 5): [Product] topCars(first: Int = 5): [Car] } @@ -65,11 +66,32 @@ export const typeDefs = gql` details: ProductDetailsBook } - type Car @key(fields: "id") { + interface Vehicle { id: String! + description: String price: String } + type Car implements Vehicle @key(fields: "id") { + id: String! + description: String + price: String + } + + type Van implements Vehicle @key(fields: "id") { + id: String! + description: String + price: String + } + + union Thing = Car | Ikea + + extend type User @key(fields: "id") { + id: ID! @external + vehicle: Vehicle + thing: Thing + } + # Value type type KeyValue { key: String! @@ -131,17 +153,25 @@ const products = [ { __typename: 'Book', isbn: '0987654321', price: 29 }, ]; -const cars = [ +const vehicles = [ { __typename: 'Car', id: '1', + description: 'Humble Toyota', price: 9990, }, { __typename: 'Car', id: '2', + description: 'Awesome Tesla', price: 12990, }, + { + __typename: 'Van', + id: '3', + description: 'Just a van...', + price: 15990, + }, ]; export const resolvers: GraphQLResolverMap = { @@ -176,13 +206,34 @@ export const resolvers: GraphQLResolverMap = { }, Car: { __resolveReference(object) { - return cars.find(car => car.id === object.id); + return vehicles.find(vehicles => vehicles.id === object.id); + }, + }, + Van: { + __resolveReference(object) { + return vehicles.find(vehicles => vehicles.id === object.id); + }, + }, + Thing: { + __resolveType(object) { + return 'id' in object ? 'Car' : 'Ikea'; + }, + }, + User: { + vehicle(user) { + return vehicles.find(vehicles => vehicles.id === user.id); + }, + thing(user) { + return vehicles.find(vehicles => vehicles.id === user.id); }, }, Query: { product(_, args) { return products.find(product => product.upc === args.upc); }, + vehicle(_, args) { + return vehicles.find(vehicles => vehicles.id === args.id); + }, topProducts(_, args) { return products.slice(0, args.first); }, diff --git a/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/reviews.ts b/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/reviews.ts index 53a399bd0d3..69b8f58ef90 100644 --- a/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/reviews.ts +++ b/packages/apollo-gateway/src/__tests__/__fixtures__/schemas/reviews.ts @@ -52,7 +52,17 @@ export const typeDefs = gql` relatedReviews: [Review!]! @requires(fields: "similarBooks { isbn }") } - extend type Car @key(fields: "id") { + extend interface Vehicle { + retailPrice: String + } + + extend type Car implements Vehicle @key(fields: "id") { + id: String! @external + price: String @external + retailPrice: String @requires(fields: "price") + } + + extend type Van implements Vehicle @key(fields: "id") { id: String! @external price: String @external retailPrice: String @requires(fields: "price") @@ -217,6 +227,11 @@ export const resolvers: GraphQLResolverMap = { return car.price; }, }, + Van: { + retailPrice(van) { + return van.price; + }, + }, MetadataOrError: { __resolveType(object) { return 'key' in object ? 'KeyValue' : 'Error'; diff --git a/packages/apollo-gateway/src/__tests__/executeQueryPlan.test.ts b/packages/apollo-gateway/src/__tests__/executeQueryPlan.test.ts index 6d0d872247e..2069f88df90 100644 --- a/packages/apollo-gateway/src/__tests__/executeQueryPlan.test.ts +++ b/packages/apollo-gateway/src/__tests__/executeQueryPlan.test.ts @@ -15,6 +15,10 @@ import { buildQueryPlan, buildOperationContext } from '../buildQueryPlan'; import { executeQueryPlan } from '../executeQueryPlan'; import { LocalGraphQLDataSource } from '../datasources/LocalGraphQLDataSource'; +import { astSerializer, queryPlanSerializer } from '../snapshotSerializers'; +expect.addSnapshotSerializer(astSerializer); +expect.addSnapshotSerializer(queryPlanSerializer); + function buildLocalService(modules: GraphQLSchemaModule[]) { const schema = buildFederatedSchema(modules); return new LocalGraphQLDataSource(schema); @@ -434,6 +438,119 @@ describe('executeQueryPlan', () => { expect(response.errors).toBeUndefined(); }); + it(`can execute queries on interface types`, async () => { + const query = gql` + query { + vehicle(id: "1") { + description + price + retailPrice + } + } + `; + + const operationContext = buildOperationContext(schema, query); + const queryPlan = buildQueryPlan(operationContext); + + const response = await executeQueryPlan( + queryPlan, + serviceMap, + buildRequestContext(), + operationContext, + ); + + expect(response.data).toMatchInlineSnapshot(` + Object { + "vehicle": Object { + "description": "Humble Toyota", + "price": "9990", + "retailPrice": "9990", + }, + } + `); + }); + + it(`can execute queries whose fields are interface types`, async () => { + const query = gql` + query { + user(id: "1") { + name + vehicle { + description + price + retailPrice + } + } + } + `; + + const operationContext = buildOperationContext(schema, query); + const queryPlan = buildQueryPlan(operationContext); + + const response = await executeQueryPlan( + queryPlan, + serviceMap, + buildRequestContext(), + operationContext, + ); + + expect(response.data).toMatchInlineSnapshot(` + Object { + "user": Object { + "name": "Ada Lovelace", + "vehicle": Object { + "description": "Humble Toyota", + "price": "9990", + "retailPrice": "9990", + }, + }, + } + `); + }); + + it(`can execute queries whose fields are union types`, async () => { + const query = gql` + query { + user(id: "1") { + name + thing { + ... on Vehicle { + description + price + retailPrice + } + ... on Ikea { + asile + } + } + } + } + `; + + const operationContext = buildOperationContext(schema, query); + const queryPlan = buildQueryPlan(operationContext); + + const response = await executeQueryPlan( + queryPlan, + serviceMap, + buildRequestContext(), + operationContext, + ); + + expect(response.data).toMatchInlineSnapshot(` + Object { + "user": Object { + "name": "Ada Lovelace", + "thing": Object { + "description": "Humble Toyota", + "price": "9990", + "retailPrice": "9990", + }, + }, + } + `); + }); + it('can execute queries with falsey @requires (except undefined)', async () => { const query = gql` query {