diff --git a/packages/apollo-federation/src/composition/__tests__/compose.test.ts b/packages/apollo-federation/src/composition/__tests__/compose.test.ts index 40c001c4fcf..c233d237933 100644 --- a/packages/apollo-federation/src/composition/__tests__/compose.test.ts +++ b/packages/apollo-federation/src/composition/__tests__/compose.test.ts @@ -1,4 +1,4 @@ -import { GraphQLObjectType, GraphQLEnumType } from 'graphql'; +import { GraphQLObjectType } from 'graphql'; import gql from 'graphql-tag'; import { composeServices } from '../compose'; import { @@ -7,6 +7,7 @@ import { selectionSetSerializer, } from '../../snapshotSerializers'; import { normalizeTypeDefs } from '../normalize'; +import { getOwningService } from '../utils'; expect.addSnapshotSerializer(astSerializer); expect.addSnapshotSerializer(typeSerializer); @@ -55,8 +56,8 @@ describe('composeServices', () => { const product = schema.getType('Product') as GraphQLObjectType; const user = schema.getType('User') as GraphQLObjectType; - expect(product.federation.serviceName).toEqual('serviceA'); - expect(user.federation.serviceName).toEqual('serviceB'); + expect(getOwningService(product)).toEqual('serviceA'); + expect(getOwningService(user)).toEqual('serviceB'); }); describe('basic type extensions', () => { @@ -94,7 +95,7 @@ describe('composeServices', () => { const product = schema.getType('Product') as GraphQLObjectType; - expect(product.federation.serviceName).toEqual('serviceA'); + expect(getOwningService(product)).toEqual('serviceA'); expect(product.getFields()['price'].federation.serviceName).toEqual( 'serviceB', ); @@ -133,7 +134,7 @@ describe('composeServices', () => { const product = schema.getType('Product') as GraphQLObjectType; - expect(product.federation.serviceName).toEqual('serviceB'); + expect(getOwningService(product)).toEqual('serviceB'); expect(product.getFields()['price'].federation.serviceName).toEqual( 'serviceA', ); @@ -187,7 +188,7 @@ describe('composeServices', () => { const product = schema.getType('Product') as GraphQLObjectType; - expect(product.federation.serviceName).toEqual('serviceB'); + expect(getOwningService(product)).toEqual('serviceB'); expect(product.getFields()['price'].federation.serviceName).toEqual( 'serviceA', ); @@ -248,7 +249,7 @@ describe('composeServices', () => { } `); - expect(product.federation.serviceName).toEqual('serviceB'); + expect(getOwningService(product)).toEqual('serviceB'); expect(product.getFields()['price'].federation.serviceName).toEqual( 'serviceC', ); @@ -574,7 +575,7 @@ describe('composeServices', () => { const product = schema.getType('Product') as GraphQLObjectType; - expect(product.federation.serviceName).toEqual('serviceA'); + expect(getOwningService(product)).toEqual('serviceA'); expect(product.getFields()['id'].federation.serviceName).toEqual( 'serviceB', ); @@ -614,7 +615,7 @@ describe('composeServices', () => { const query = schema.getQueryType(); - expect(query.federation.serviceName).toBeUndefined(); + expect(getOwningService(query)).toBeUndefined(); }); it('treats root Query type definition as an extension, not base definitions', () => { @@ -655,7 +656,7 @@ describe('composeServices', () => { const query = schema.getType('Query') as GraphQLObjectType; - expect(query.federation.serviceName).toBeUndefined(); + expect(getOwningService(query)).toBeUndefined(); }); it('allows extension of the Mutation type with no base type definition', () => { @@ -846,7 +847,7 @@ describe('composeServices', () => { expect(product.getFields()['price'].federation.serviceName).toEqual( 'serviceB', ); - expect(product.federation.serviceName).toEqual('serviceA'); + expect(getOwningService(product)).toEqual('serviceA'); }); }); diff --git a/packages/apollo-federation/src/composition/compose.ts b/packages/apollo-federation/src/composition/compose.ts index a65c2d2a9bf..7b4965caea9 100644 --- a/packages/apollo-federation/src/composition/compose.ts +++ b/packages/apollo-federation/src/composition/compose.ts @@ -27,7 +27,6 @@ import { parseSelections, mapFieldNamesToServiceName, stripExternalFieldsFromTypeDefs, - diffTypeNodes, } from './utils'; import { ServiceDefinition, @@ -80,7 +79,7 @@ interface ExtensionsMap { */ interface TypeToServiceMap { [typeName: string]: { - serviceName?: ServiceName; + serviceNames?: ServiceName[]; extensionFieldsToOwningServiceMap: { [fieldName: string]: string }; }; } @@ -158,10 +157,13 @@ export function buildMapsFromServiceList(serviceList: ServiceDefinition[]) { * 2. It was extended by a service before declared */ if (typeToServiceMap[typeName]) { - typeToServiceMap[typeName].serviceName = serviceName; + typeToServiceMap[typeName].serviceNames = [ + ...(typeToServiceMap[typeName].serviceNames || []), + serviceName, + ]; } else { typeToServiceMap[typeName] = { - serviceName, + serviceNames: [serviceName], extensionFieldsToOwningServiceMap: Object.create(null), }; } @@ -314,7 +316,7 @@ export function addFederationMetadataToSchemaNodes({ }) { for (const [ typeName, - { serviceName: baseServiceName, extensionFieldsToOwningServiceMap }, + { serviceNames, extensionFieldsToOwningServiceMap }, ] of Object.entries(typeToServiceMap)) { const namedType = schema.getType(typeName) as GraphQLNamedType; if (!namedType) continue; @@ -323,7 +325,7 @@ export function addFederationMetadataToSchemaNodes({ // and the key directives that belong to it namedType.federation = { ...namedType.federation, - serviceName: baseServiceName, + serviceNames, ...(keyDirectivesMap[typeName] && { keys: keyDirectivesMap[typeName], }), @@ -344,7 +346,7 @@ export function addFederationMetadataToSchemaNodes({ ) { field.federation = { ...field.federation, - serviceName: baseServiceName, + serviceName: serviceNames && serviceNames[serviceNames.length - 1], provides: parseSelections( providesDirective.arguments[0].value.value, ), @@ -420,38 +422,6 @@ export function composeServices(services: ServiceDefinition[]) { keyDirectivesMap, } = buildMapsFromServiceList(services); - for (const [typeName, definitions] of Object.entries(definitionsMap)) { - const typeIsValueType = - definitions.length > 1 && - definitions.every((definition, index) => { - if (findDirectivesOnTypeOrField(definition, 'key').length > 0) { - return false; - } - if (index === 0) { - return true; - } - - const { name, kind, fields, unionTypes } = diffTypeNodes( - definition, - definitions[index - 1], - ); - - return ( - name.length === 0 && - kind.length === 0 && - Object.keys(fields).length === 0 && - Object.keys(unionTypes).length === 0 - ); - }); - - if (typeIsValueType) { - definitionsMap[typeName] = [ - { ...definitionsMap[typeName][0], serviceName: null }, - ]; - typeToServiceMap[typeName].serviceName = null; - } - } - let { schema, errors } = buildSchemaFromDefinitionsAndExtensions({ definitionsMap, extensionsMap, diff --git a/packages/apollo-federation/src/composition/types.ts b/packages/apollo-federation/src/composition/types.ts index 50c5b44e62d..820e4b57bf3 100644 --- a/packages/apollo-federation/src/composition/types.ts +++ b/packages/apollo-federation/src/composition/types.ts @@ -18,7 +18,7 @@ export interface ServiceNameToKeyDirectivesMap { } export interface FederationType { - serviceName?: ServiceName; + serviceNames?: ServiceName[]; keys?: ServiceNameToKeyDirectivesMap; externals?: { [serviceName: string]: ExternalFieldDefinition[]; diff --git a/packages/apollo-federation/src/composition/utils.ts b/packages/apollo-federation/src/composition/utils.ts index 2737e6d8fa7..52eb65afdc0 100644 --- a/packages/apollo-federation/src/composition/utils.ts +++ b/packages/apollo-federation/src/composition/utils.ts @@ -181,6 +181,12 @@ export function errorWithCode( ); } +export function getOwningService(type: GraphQLNamedType) { + return type.federation && type.federation.serviceNames + ? type.federation.serviceNames[type.federation.serviceNames.length - 1] + : undefined; +} + export function findTypesContainingFieldWithReturnType( schema: GraphQLSchema, node: GraphQLField, diff --git a/packages/apollo-federation/src/composition/validate/postComposition/externalMissingOnBase.ts b/packages/apollo-federation/src/composition/validate/postComposition/externalMissingOnBase.ts index 92ded43d58c..db40db2a5b4 100644 --- a/packages/apollo-federation/src/composition/validate/postComposition/externalMissingOnBase.ts +++ b/packages/apollo-federation/src/composition/validate/postComposition/externalMissingOnBase.ts @@ -1,7 +1,11 @@ import 'apollo-server-env'; import { GraphQLSchema, isObjectType, GraphQLError } from 'graphql'; -import { logServiceAndType, errorWithCode } from '../../utils'; +import { + logServiceAndType, + errorWithCode, + getOwningService, +} from '../../utils'; /** * All fields marked with @external must exist on the base type @@ -33,7 +37,9 @@ export const externalMissingOnBase = (schema: GraphQLSchema) => { errorWithCode( 'EXTERNAL_MISSING_ON_BASE', logServiceAndType(serviceName, typeName, externalFieldName) + - `marked @external but ${externalFieldName} is not defined on the base service of ${typeName} (${namedType.federation.serviceName})`, + `marked @external but ${externalFieldName} is not defined on the base service of ${typeName} (${getOwningService( + namedType, + )})`, ), ); continue; @@ -49,7 +55,11 @@ export const externalMissingOnBase = (schema: GraphQLSchema) => { errorWithCode( 'EXTERNAL_MISSING_ON_BASE', logServiceAndType(serviceName, typeName, externalFieldName) + - `marked @external but ${externalFieldName} was defined in ${matchingBaseField.federation.serviceName}, not in the service that owns ${typeName} (${namedType.federation.serviceName})`, + `marked @external but ${externalFieldName} was defined in ${ + matchingBaseField.federation.serviceName + }, not in the service that owns ${typeName} (${getOwningService( + namedType, + )})`, ), ); } diff --git a/packages/apollo-federation/src/composition/validate/postComposition/externalTypeMismatch.ts b/packages/apollo-federation/src/composition/validate/postComposition/externalTypeMismatch.ts index a5bda47baad..922a5c847ad 100644 --- a/packages/apollo-federation/src/composition/validate/postComposition/externalTypeMismatch.ts +++ b/packages/apollo-federation/src/composition/validate/postComposition/externalTypeMismatch.ts @@ -5,7 +5,11 @@ import { isEqualType, GraphQLError, } from 'graphql'; -import { logServiceAndType, errorWithCode } from '../../utils'; +import { + logServiceAndType, + errorWithCode, + getOwningService, +} from '../../utils'; /** * All fields marked with @external must match the type definition of the base service. @@ -56,7 +60,11 @@ export const externalTypeMismatch = (schema: GraphQLSchema) => { errorWithCode( 'EXTERNAL_TYPE_MISMATCH', logServiceAndType(serviceName, typeName, externalFieldName) + - `Type \`${externalFieldType.name}\` does not match the type of the original field in ${namedType.federation.serviceName} (\`${matchingBaseField.type}\`)`, + `Type \`${ + externalFieldType.name + }\` does not match the type of the original field in ${getOwningService( + namedType, + )} (\`${matchingBaseField.type}\`)`, ), ); } diff --git a/packages/apollo-gateway/src/__tests__/integration/value-types.test.ts b/packages/apollo-gateway/src/__tests__/integration/value-types.test.ts index 3f92404102f..db5dea00b1c 100644 --- a/packages/apollo-gateway/src/__tests__/integration/value-types.test.ts +++ b/packages/apollo-gateway/src/__tests__/integration/value-types.test.ts @@ -35,7 +35,7 @@ describe('value types', () => { } `; - const { data, errors, queryPlan } = await execute( + const { data, errors } = await execute( [accounts, books, inventory, product, reviews], { query, @@ -43,13 +43,13 @@ describe('value types', () => { ); expect(errors).toBeUndefined(); - expect(data.topProducts[0].upc).toEqual('1'); - expect(data.topProducts[0].metadata[0]).toEqual({ + expect(data!.topProducts[0].upc).toEqual('1'); + expect(data!.topProducts[0].metadata[0]).toEqual({ key: 'Condition', value: 'excellent', }); - expect(data.topProducts[4].upc).toEqual('0136291554'); - expect(data.topProducts[4].metadata[0]).toEqual({ + expect(data!.topProducts[4].upc).toEqual('0136291554'); + expect(data!.topProducts[4].metadata[0]).toEqual({ key: 'Condition', value: 'used', }); diff --git a/packages/apollo-gateway/src/buildQueryPlan.ts b/packages/apollo-gateway/src/buildQueryPlan.ts index 3af5c5ab0ba..bc2b93ef215 100644 --- a/packages/apollo-gateway/src/buildQueryPlan.ts +++ b/packages/apollo-gateway/src/buildQueryPlan.ts @@ -253,12 +253,23 @@ function splitSubfields( splitFields(context, path, fields, field => { const { parentType, fieldNode, fieldDef } = field; - const owningService = - context.getOwningService(parentType, fieldDef) || parentGroup.serviceName; + const baseService = context.getBaseService(parentType); - const baseService = - context.getBaseService(parentType) || parentGroup.serviceName; + if (!baseService) { + throw new GraphQLError( + `Couldn't find base service for type "${parentType.name}"`, + fieldNode, + ); + } + const owningService = context.getOwningService(parentType, fieldDef); + + if (!owningService) { + throw new GraphQLError( + `Couldn't find owning service for field "${parentType.name}.${fieldDef.name}"`, + fieldNode, + ); + } // Is the field defined on the base service? if (owningService === baseService) { // Can we fetch the field from the parent group? @@ -735,7 +746,14 @@ export class QueryPlanningContext { } getBaseService(parentType: GraphQLObjectType): string | null { - return (parentType.federation && parentType.federation.serviceName) || null; + return ( + (parentType.federation && + parentType.federation.serviceNames && + parentType.federation.serviceNames[ + parentType.federation.serviceNames.length - 1 + ]) || + null + ); } getOwningService(