From 51ce6a2b51ef71ca7f1f543e076a2507e78f8303 Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Thu, 13 Jun 2019 17:51:42 -0700 Subject: [PATCH 1/2] Add support for list type keys in federation. --- .../keyFieldsSelectInvalidType.test.ts | 47 ------- .../keyFieldsSelectInvalidType.ts | 14 -- .../__tests__/integration/list-key.test.ts | 125 ++++++++++++++++++ 3 files changed, 125 insertions(+), 61 deletions(-) create mode 100644 packages/apollo-gateway/src/__tests__/integration/list-key.test.ts diff --git a/packages/apollo-federation/src/composition/validate/postComposition/__tests__/keyFieldsSelectInvalidType.test.ts b/packages/apollo-federation/src/composition/validate/postComposition/__tests__/keyFieldsSelectInvalidType.test.ts index d6c65af26c0..ab4de14d0a3 100644 --- a/packages/apollo-federation/src/composition/validate/postComposition/__tests__/keyFieldsSelectInvalidType.test.ts +++ b/packages/apollo-federation/src/composition/validate/postComposition/__tests__/keyFieldsSelectInvalidType.test.ts @@ -42,53 +42,6 @@ describe('keyFieldsSelectInvalidType', () => { expect(warnings).toHaveLength(0); }); - it('warns if @key references fields of a list type', () => { - const serviceA = { - typeDefs: gql` - type Product @key(fields: "myList myOptionalList") { - sku: String! - myList: [String]! - myOptionalList: [String] - upc: String! - color: Color! - } - - type Color { - id: ID! - value: String! - } - `, - name: 'serviceA', - }; - - const serviceB = { - typeDefs: gql` - extend type Product { - sku: String! @external - price: Int! @requires(fields: "sku") - } - `, - name: 'serviceB', - }; - - const { schema, errors } = composeServices([serviceA, serviceB]); - expect(errors).toHaveLength(0); - - const warnings = validateKeyFieldsSelectInvalidType(schema); - expect(warnings).toMatchInlineSnapshot(` - Array [ - Object { - "code": "KEY_FIELDS_SELECT_INVALID_TYPE", - "message": "[serviceA] Product -> A @key selects Product.myList, which is a list type. Keys cannot select lists.", - }, - Object { - "code": "KEY_FIELDS_SELECT_INVALID_TYPE", - "message": "[serviceA] Product -> A @key selects Product.myOptionalList, which is a list type. Keys cannot select lists.", - }, - ] - `); - }); - it('warns if @key references fields of an interface type', () => { const serviceA = { typeDefs: gql` diff --git a/packages/apollo-federation/src/composition/validate/postComposition/keyFieldsSelectInvalidType.ts b/packages/apollo-federation/src/composition/validate/postComposition/keyFieldsSelectInvalidType.ts index bb6801cebdf..e0db456ad1e 100644 --- a/packages/apollo-federation/src/composition/validate/postComposition/keyFieldsSelectInvalidType.ts +++ b/packages/apollo-federation/src/composition/validate/postComposition/keyFieldsSelectInvalidType.ts @@ -2,7 +2,6 @@ import { GraphQLSchema, isObjectType, FieldNode, - isListType, isInterfaceType, isNonNullType, getNullableType, @@ -46,19 +45,6 @@ export const keyFieldsSelectInvalidType = (schema: GraphQLSchema) => { } if (matchingField) { - if ( - isListType(matchingField.type) || - (isNonNullType(matchingField.type) && - isListType(getNullableType(matchingField.type))) - ) { - errors.push( - errorWithCode( - 'KEY_FIELDS_SELECT_INVALID_TYPE', - logServiceAndType(serviceName, typeName) + - `A @key selects ${typeName}.${name}, which is a list type. Keys cannot select lists.`, - ), - ); - } if ( isInterfaceType(matchingField.type) || (isNonNullType(matchingField.type) && diff --git a/packages/apollo-gateway/src/__tests__/integration/list-key.test.ts b/packages/apollo-gateway/src/__tests__/integration/list-key.test.ts new file mode 100644 index 00000000000..e0b38657987 --- /dev/null +++ b/packages/apollo-gateway/src/__tests__/integration/list-key.test.ts @@ -0,0 +1,125 @@ +import gql from 'graphql-tag'; +import { execute, ServiceDefinitionModule } from '../execution-utils'; +import { astSerializer, queryPlanSerializer } from '../../snapshotSerializers'; + +expect.addSnapshotSerializer(astSerializer); +expect.addSnapshotSerializer(queryPlanSerializer); + +const users = [ + { id: ['1', '1'], name: 'Trevor Scheer', __typename: 'User' }, + { id: ['2', '2'], name: 'James Baxley', __typename: 'User' }, +]; + +const reviews = [ + { id: '1', authorId: ['1', '1'], body: 'Good', __typename: 'Review' }, + { id: '2', authorId: ['2', '2'], body: 'Bad', __typename: 'Review' }, +]; + +const reviewService: ServiceDefinitionModule = { + name: 'review', + typeDefs: gql` + type Query { + reviews: [Review!]! + } + + type Review { + id: ID! + author: User! + body: String! + } + + extend type User @key(fields: "id") { + id: [ID!]! @external + } + `, + resolvers: { + Query: { + reviews() { + return reviews; + }, + }, + Review: { + author(review) { + return { + id: review.authorId, + }; + }, + }, + }, +}; + +const listsAreEqual = (as: T[], bs: T[]) => + as.length === bs.length && as.every((a, i) => bs[i] === as[i]); + +const userService: ServiceDefinitionModule = { + name: 'user', + typeDefs: gql` + type User @key(fields: "id") { + id: [ID!]! + name: String! + } + `, + resolvers: { + User: { + __resolveReference(reference) { + return users.find(user => listsAreEqual(user.id, reference.id)); + }, + }, + }, +}; + +it('fetches data correctly list type @key fields', async () => { + const query = gql` + query Reviews { + reviews { + body + author { + name + } + } + } + `; + + const { data, queryPlan } = await execute([userService, reviewService], { + query, + }); + + expect(data).toEqual({ + reviews: [ + { body: 'Good', author: { name: 'Trevor Scheer' } }, + { body: 'Bad', author: { name: 'James Baxley' } }, + ], + }); + expect(queryPlan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "review") { + { + reviews { + body + author { + __typename + id + } + } + } + }, + Flatten(path: "reviews.@.author") { + Fetch(service: "user") { + { + ... on User { + __typename + id + } + } => + { + ... on User { + name + } + } + }, + }, + }, + } + `); +}); From b615c6976083b76d2d97bb2ff035816c2029b450 Mon Sep 17 00:00:00 2001 From: Jackson Kearl Date: Thu, 13 Jun 2019 17:53:12 -0700 Subject: [PATCH 2/2] Update docs --- docs/source/federation/advanced-features.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/federation/advanced-features.md b/docs/source/federation/advanced-features.md index 0b64e2d5ac8..6802ee24181 100644 --- a/docs/source/federation/advanced-features.md +++ b/docs/source/federation/advanced-features.md @@ -54,7 +54,7 @@ type Organization { } ``` -> Note that although the fields argument is parsed as a selection set, some restrictions apply to make the result suitable as a key. For example, fields shouldn't return lists. +> Note that although the fields argument is parsed as a selection set, some restrictions apply to make the result suitable as a key. For example, fields shouldn't return unions or interfaces. ## Computed fields