From ceee50b44e6a26069cd8c31244a36995db21d7be Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 20 Jun 2020 23:12:07 -0400 Subject: [PATCH 1/5] some fields may only be on gateway ie extensions, so additional null checks required initial change necessary for #1662 --- packages/delegate/src/mergeFields.ts | 55 ++++++++++++++++------------ 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/packages/delegate/src/mergeFields.ts b/packages/delegate/src/mergeFields.ts index 064b74eec22..b30722e7160 100644 --- a/packages/delegate/src/mergeFields.ts +++ b/packages/delegate/src/mergeFields.ts @@ -48,33 +48,42 @@ function buildDelegationPlan( const uniqueSubschema: SubschemaConfig = uniqueFields[selection.name.value]; if (uniqueSubschema != null) { - if (proxiableSubschemas.includes(uniqueSubschema)) { - const existingSubschema = delegationMap.get(uniqueSubschema); - if (existingSubschema != null) { - existingSubschema.push(selection); - } else { - delegationMap.set(uniqueSubschema, [selection]); - } - } else { + if (!proxiableSubschemas.includes(uniqueSubschema)) { unproxiableSelections.push(selection); + return; } - } else { - // 2b. use nonUniqueFields to assign to a possible subschema, - // preferring one of the subschemas already targets of delegation - - let nonUniqueSubschemas: Array = nonUniqueFields[selection.name.value]; - nonUniqueSubschemas = nonUniqueSubschemas.filter(s => proxiableSubschemas.includes(s)); - if (nonUniqueSubschemas != null) { - const subschemas: Array = Array.from(delegationMap.keys()); - const existingSubschema = nonUniqueSubschemas.find(s => subschemas.includes(s)); - if (existingSubschema != null) { - delegationMap.get(existingSubschema).push(selection); - } else { - delegationMap.set(nonUniqueSubschemas[0], [selection]); - } + + const existingSubschema = delegationMap.get(uniqueSubschema); + if (existingSubschema != null) { + existingSubschema.push(selection); } else { - unproxiableSelections.push(selection); + delegationMap.set(uniqueSubschema, [selection]); } + + return; + } + + // 2b. use nonUniqueFields to assign to a possible subschema, + // preferring one of the subschemas already targets of delegation + + let nonUniqueSubschemas: Array = nonUniqueFields[selection.name.value]; + if (nonUniqueSubschemas == null) { + unproxiableSelections.push(selection); + return; + } + + nonUniqueSubschemas = nonUniqueSubschemas.filter(s => proxiableSubschemas.includes(s)); + if (nonUniqueSubschemas == null) { + unproxiableSelections.push(selection); + return; + } + + const subschemas: Array = Array.from(delegationMap.keys()); + const existingSubschema = nonUniqueSubschemas.find(s => subschemas.includes(s)); + if (existingSubschema != null) { + delegationMap.get(existingSubschema).push(selection); + } else { + delegationMap.set(nonUniqueSubschemas[0], [selection]); } }); From 069eb6d47ba5d1607de15755b4fd7cc41e6b504c Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 20 Jun 2020 23:25:18 -0400 Subject: [PATCH 2/5] add failing test --- packages/stitch/tests/typeMerging.test.ts | 132 ++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/packages/stitch/tests/typeMerging.test.ts b/packages/stitch/tests/typeMerging.test.ts index ad48ddc6b01..16e83b3338e 100644 --- a/packages/stitch/tests/typeMerging.test.ts +++ b/packages/stitch/tests/typeMerging.test.ts @@ -7,6 +7,8 @@ import { makeExecutableSchema } from '@graphql-tools/schema'; import { addMocksToSchema } from '@graphql-tools/mock'; +import { delegateToSchema } from '@graphql-tools/delegate'; + import { stitchSchemas } from '../src/stitchSchemas'; let chirpSchema = makeExecutableSchema({ @@ -103,3 +105,133 @@ describe('merging using type merging', () => { expect(result.data.userById.chirps[1].author.email).not.toBe(null); }); }); + +describe('merge types and extend', () => { + test('should work', async () => { + const resultSchema = makeExecutableSchema({ + typeDefs: ` + type Query { + resultById(id: ID!): String + } + `, + resolvers: { + Query: { + resultById: () => 'ok', + }, + }, + }); + + const containerSchemaA = makeExecutableSchema({ + typeDefs: ` + type Container { + id: ID! + resultId: ID! + } + + type Query { + containerById(id: ID!): Container + } + `, + resolvers: { + Query: { + containerById: () => ({ id: 'Container', resultId: 'Result' }), + }, + }, + }); + + const containerSchemaB = makeExecutableSchema({ + typeDefs: ` + type Container { + id: ID! + } + + type Query { + containerById(id: ID!): Container + rootContainer: Container! + } + `, + resolvers: { + Query: { + containerById: () => ({ id: 'Container' }), + rootContainer: () => ({ id: 'Container' }), + }, + }, + }); + + const schema = stitchSchemas({ + subschemas: [ + { + schema: resultSchema, + }, + { + schema: containerSchemaA, + merge: { + Container: { + fieldName: 'containerById', + args: ({ id }) => ({ id }), + selectionSet: '{ id }', + }, + }, + }, + { + schema: containerSchemaB, + merge: { + Container: { + fieldName: 'containerById', + args: ({ id }) => ({ id }), + selectionSet: '{ id }', + }, + }, + }, + ], + mergeTypes: true, + typeDefs: ` + extend type Container { + result: String! + } + `, + resolvers: { + Container: { + result: { + selectionSet: `{ resultId }`, + resolve(container, _args, context, info) { + return delegateToSchema({ + schema: resultSchema, + operation: 'query', + fieldName: 'resultById', + args: { + id: container.resultId, + }, + context, + info, + }); + }, + }, + }, + }, + }); + + const result = await graphql( + schema, + ` + query TestQuery { + rootContainer { + id + result + } + } + `, + ); + + const expectedResult = { + data: { + rootContainer: { + id: 'Container', + result: 'ok', + } + } + } + + expect(result).toEqual(expectedResult); + }) +}) From 758fc3483a5eb2a7590843cd92952b5f3758d881 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 21 Jun 2020 03:55:10 -0400 Subject: [PATCH 3/5] correct use of term selections calling collectFields replaces selections with actual fieldNodes --- packages/delegate/src/mergeFields.ts | 42 +++++++++---------- packages/delegate/src/results/handleObject.ts | 6 +-- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/packages/delegate/src/mergeFields.ts b/packages/delegate/src/mergeFields.ts index b30722e7160..6b07b426832 100644 --- a/packages/delegate/src/mergeFields.ts +++ b/packages/delegate/src/mergeFields.ts @@ -5,12 +5,12 @@ import { MergedTypeInfo, SubschemaConfig } from './types'; function buildDelegationPlan( mergedTypeInfo: MergedTypeInfo, - originalSelections: Array, + fieldNodes: Array, sourceSubschemas: Array, targetSubschemas: Array ): { delegationMap: Map>; - unproxiableSelections: Array; + unproxiableFieldNodes: Array; proxiableSubschemas: Array; nonProxiableSubschemas: Array; } { @@ -34,30 +34,30 @@ function buildDelegationPlan( }); const { uniqueFields, nonUniqueFields } = mergedTypeInfo; - const unproxiableSelections: Array = []; + const unproxiableFieldNodes: Array = []; // 2. for each selection: const delegationMap: Map> = new Map(); - originalSelections.forEach(selection => { - if (selection.name.value === '__typename') { + fieldNodes.forEach(fieldNode => { + if (fieldNode.name.value === '__typename') { return; } // 2a. use uniqueFields map to assign fields to subschema if one of possible subschemas - const uniqueSubschema: SubschemaConfig = uniqueFields[selection.name.value]; + const uniqueSubschema: SubschemaConfig = uniqueFields[fieldNode.name.value]; if (uniqueSubschema != null) { if (!proxiableSubschemas.includes(uniqueSubschema)) { - unproxiableSelections.push(selection); + unproxiableFieldNodes.push(fieldNode); return; } const existingSubschema = delegationMap.get(uniqueSubschema); if (existingSubschema != null) { - existingSubschema.push(selection); + existingSubschema.push(fieldNode); } else { - delegationMap.set(uniqueSubschema, [selection]); + delegationMap.set(uniqueSubschema, [fieldNode]); } return; @@ -66,30 +66,30 @@ function buildDelegationPlan( // 2b. use nonUniqueFields to assign to a possible subschema, // preferring one of the subschemas already targets of delegation - let nonUniqueSubschemas: Array = nonUniqueFields[selection.name.value]; + let nonUniqueSubschemas: Array = nonUniqueFields[fieldNode.name.value]; if (nonUniqueSubschemas == null) { - unproxiableSelections.push(selection); + unproxiableFieldNodes.push(fieldNode); return; } nonUniqueSubschemas = nonUniqueSubschemas.filter(s => proxiableSubschemas.includes(s)); if (nonUniqueSubschemas == null) { - unproxiableSelections.push(selection); + unproxiableFieldNodes.push(fieldNode); return; } const subschemas: Array = Array.from(delegationMap.keys()); const existingSubschema = nonUniqueSubschemas.find(s => subschemas.includes(s)); if (existingSubschema != null) { - delegationMap.get(existingSubschema).push(selection); + delegationMap.get(existingSubschema).push(fieldNode); } else { - delegationMap.set(nonUniqueSubschemas[0], [selection]); + delegationMap.set(nonUniqueSubschemas[0], [fieldNode]); } }); return { delegationMap, - unproxiableSelections, + unproxiableFieldNodes, proxiableSubschemas, nonProxiableSubschemas, }; @@ -99,19 +99,19 @@ export function mergeFields( mergedTypeInfo: MergedTypeInfo, typeName: string, object: any, - originalSelections: Array, + fieldNodes: Array, sourceSubschemas: Array, targetSubschemas: Array, context: Record, info: GraphQLResolveInfo ): any { - if (!originalSelections.length) { + if (!fieldNodes.length) { return object; } - const { delegationMap, unproxiableSelections, proxiableSubschemas, nonProxiableSubschemas } = buildDelegationPlan( + const { delegationMap, unproxiableFieldNodes, proxiableSubschemas, nonProxiableSubschemas } = buildDelegationPlan( mergedTypeInfo, - originalSelections, + fieldNodes, sourceSubschemas, targetSubschemas ); @@ -143,7 +143,7 @@ export function mergeFields( mergedTypeInfo, typeName, mergeProxiedResults(object, ...results), - unproxiableSelections, + unproxiableFieldNodes, sourceSubschemas.concat(proxiableSubschemas), nonProxiableSubschemas, context, @@ -154,7 +154,7 @@ export function mergeFields( mergedTypeInfo, typeName, mergeProxiedResults(object, ...maybePromises), - unproxiableSelections, + unproxiableFieldNodes, sourceSubschemas.concat(proxiableSubschemas), nonProxiableSubschemas, context, diff --git a/packages/delegate/src/results/handleObject.ts b/packages/delegate/src/results/handleObject.ts index 51d18e0d66a..5be99955ca4 100644 --- a/packages/delegate/src/results/handleObject.ts +++ b/packages/delegate/src/results/handleObject.ts @@ -54,13 +54,13 @@ export function handleObject( const subFields = collectSubFields(info, object.__typename); - const selections = getFieldsNotInSubschema(subFields, subschema, mergedTypeInfo, object.__typename); + const fieldNodes = getFieldsNotInSubschema(subFields, subschema, mergedTypeInfo, object.__typename); return mergeFields( mergedTypeInfo, typeName, object, - selections, + fieldNodes, [subschema as SubschemaConfig], targetSubschemas, context, @@ -68,7 +68,7 @@ export function handleObject( ); } -function collectSubFields(info: GraphQLResolveInfo, typeName: string) { +function collectSubFields(info: GraphQLResolveInfo, typeName: string): Record> { let subFieldNodes: Record> = Object.create(null); const visitedFragmentNames = Object.create(null); info.fieldNodes.forEach(fieldNode => { From edebace79d574f620ad281a14504b51ff2aa741f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 21 Jun 2020 04:14:04 -0400 Subject: [PATCH 4/5] refactor getFieldsNotInSubschema --- packages/delegate/src/results/handleObject.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/delegate/src/results/handleObject.ts b/packages/delegate/src/results/handleObject.ts index 5be99955ca4..b7fa90e226b 100644 --- a/packages/delegate/src/results/handleObject.ts +++ b/packages/delegate/src/results/handleObject.ts @@ -52,9 +52,7 @@ export function handleObject( return object; } - const subFields = collectSubFields(info, object.__typename); - - const fieldNodes = getFieldsNotInSubschema(subFields, subschema, mergedTypeInfo, object.__typename); + const fieldNodes = getFieldsNotInSubschema(info, subschema, mergedTypeInfo, object.__typename); return mergeFields( mergedTypeInfo, @@ -88,7 +86,7 @@ function collectSubFields(info: GraphQLResolveInfo, typeName: string): Record>, + info: GraphQLResolveInfo, subschema: GraphQLSchema | SubschemaConfig, mergedTypeInfo: MergedTypeInfo, typeName: string @@ -96,13 +94,14 @@ function getFieldsNotInSubschema( const typeMap = isSubschemaConfig(subschema) ? mergedTypeInfo.typeMaps.get(subschema) : subschema.getTypeMap(); const fields = (typeMap[typeName] as GraphQLObjectType).getFields(); - const fieldsNotInSchema: Array = []; + const subFieldNodes = collectSubFields(info, typeName); + + let fieldsNotInSchema: Array = []; Object.keys(subFieldNodes).forEach(responseName => { - subFieldNodes[responseName].forEach(subFieldNode => { - if (!(subFieldNode.name.value in fields)) { - fieldsNotInSchema.push(subFieldNode); - } - }); + const fieldName = subFieldNodes[responseName][0].name.value; + if (!(fieldName in fields)) { + fieldsNotInSchema = fieldsNotInSchema.concat(subFieldNodes[responseName]); + } }); return fieldsNotInSchema; From ffe6d3c562e5ab9f6445813a88f3824122461cb4 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 21 Jun 2020 04:49:51 -0400 Subject: [PATCH 5/5] add gateway selection sets when merging types --- packages/delegate/src/results/handleObject.ts | 34 +++++++++++++++---- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/packages/delegate/src/results/handleObject.ts b/packages/delegate/src/results/handleObject.ts index b7fa90e226b..04557cb2bb1 100644 --- a/packages/delegate/src/results/handleObject.ts +++ b/packages/delegate/src/results/handleObject.ts @@ -11,7 +11,7 @@ import { import { collectFields, GraphQLExecutionContext, setErrors, slicedError } from '@graphql-tools/utils'; import { setObjectSubschema, isSubschemaConfig } from '../Subschema'; import { mergeFields } from '../mergeFields'; -import { MergedTypeInfo, SubschemaConfig } from '../types'; +import { MergedTypeInfo, SubschemaConfig, StitchingInfo } from '../types'; export function handleObject( type: GraphQLCompositeType, @@ -69,19 +69,39 @@ export function handleObject( function collectSubFields(info: GraphQLResolveInfo, typeName: string): Record> { let subFieldNodes: Record> = Object.create(null); const visitedFragmentNames = Object.create(null); + + const type = info.schema.getType(typeName) as GraphQLObjectType; + const partialExecutionContext = ({ + schema: info.schema, + variableValues: info.variableValues, + fragments: info.fragments, + } as unknown) as GraphQLExecutionContext; + info.fieldNodes.forEach(fieldNode => { subFieldNodes = collectFields( - ({ - schema: info.schema, - variableValues: info.variableValues, - fragments: info.fragments, - } as unknown) as GraphQLExecutionContext, - info.schema.getType(typeName) as GraphQLObjectType, + partialExecutionContext, + type, fieldNode.selectionSet, subFieldNodes, visitedFragmentNames ); }); + + const selectionSetsByField = (info.schema.extensions.stitchingInfo as StitchingInfo).selectionSetsByField; + + Object.keys(subFieldNodes).forEach(responseName => { + const fieldName = subFieldNodes[responseName][0].name.value; + if (selectionSetsByField[typeName] && selectionSetsByField[typeName][fieldName]) { + subFieldNodes = collectFields( + partialExecutionContext, + type, + selectionSetsByField[typeName][fieldName], + subFieldNodes, + visitedFragmentNames + ); + } + }); + return subFieldNodes; }