From 74b95c25010689033e603d43447d645b11231056 Mon Sep 17 00:00:00 2001 From: yaacovCR Date: Wed, 25 Mar 2020 01:29:00 -0400 Subject: [PATCH] fix(delegation): fix argument/variable bugs closes #46 and re-closes #44 --- src/Interfaces.ts | 22 +++ src/delegate/createRequest.ts | 265 ++++++++++++++++---------- src/delegate/delegateToSchema.ts | 2 + src/test/testAlternateMergeSchemas.ts | 52 ++++- src/test/testDelegateToSchema.ts | 96 ++++++++++ src/wrap/index.ts | 3 +- src/wrap/resolvers.ts | 16 ++ src/wrap/transformSchema.ts | 44 +---- src/wrap/wrapSchema.ts | 40 ++++ 9 files changed, 394 insertions(+), 146 deletions(-) create mode 100644 src/wrap/wrapSchema.ts diff --git a/src/Interfaces.ts b/src/Interfaces.ts index ecba7e962dd..5064d287ede 100644 --- a/src/Interfaces.ts +++ b/src/Interfaces.ts @@ -27,6 +27,7 @@ import { GraphQLFieldConfig, FragmentDefinitionNode, SelectionNode, + VariableDefinitionNode, } from 'graphql'; import { TypeMap } from 'graphql/type/schema'; @@ -236,6 +237,7 @@ export interface IDelegateToSchemaOptions { transforms?: Array; skipValidation?: boolean; skipTypeMerging?: boolean; + transformedSchema?: GraphQLSchema; } /** @@ -244,6 +246,7 @@ export interface IDelegateToSchemaOptions { export interface ICreateRequestFromInfo { info: IGraphQLToolsResolveInfo; schema: GraphQLSchema | SubschemaConfig; + transformedSchema: GraphQLSchema; operation: Operation; fieldName: string; args?: Record; @@ -251,6 +254,25 @@ export interface ICreateRequestFromInfo { fieldNodes?: ReadonlyArray; } +/** + * @category Schema Delegation + */ +export interface ICreateRequest { + sourceSchema: GraphQLSchema; + sourceParentType: GraphQLObjectType; + sourceFieldName: string; + fragments: Record; + variableDefinitions: ReadonlyArray; + variableValues: Record; + targetSchema: GraphQLSchema; + targetOperation: Operation; + targetField: string; + args: Record; + selectionSet: SelectionSetNode; + fieldNodes: ReadonlyArray; + defaultArgs: Record; +} + /** * @category Schema Delegation */ diff --git a/src/delegate/createRequest.ts b/src/delegate/createRequest.ts index db0eb17a452..d7783bcd576 100644 --- a/src/delegate/createRequest.ts +++ b/src/delegate/createRequest.ts @@ -23,10 +23,9 @@ import { import { ICreateRequestFromInfo, - Operation, Request, - SubschemaConfig, isSubschemaConfig, + ICreateRequest, } from '../Interfaces'; import { serializeInputValue } from '../utils/index'; @@ -45,43 +44,71 @@ export function getDelegatingOperation( export function createRequestFromInfo({ info, - schema, + schema: subschemaOrSubschemaConfig, + transformedSchema, operation = getDelegatingOperation(info.parentType, info.schema), fieldName = info.fieldName, args, selectionSet, fieldNodes, }: ICreateRequestFromInfo): Request { - return createRequest( - info.schema, - info.fragments, - info.operation.variableDefinitions, - info.variableValues, - schema, - operation, - fieldName, + const sourceParentType = info.parentType; + const sourceFieldName = info.fieldName; + + const fieldArguments = sourceParentType.getFields()[sourceFieldName].args; + const defaultArgs = {}; + fieldArguments.forEach((argument) => { + if (argument.defaultValue != null) { + defaultArgs[argument.name] = argument.defaultValue; + } + }); + + let targetSchema; + if (transformedSchema != null) { + targetSchema = transformedSchema; + } else { + targetSchema = isSubschemaConfig(subschemaOrSubschemaConfig) + ? subschemaOrSubschemaConfig.schema + : subschemaOrSubschemaConfig; + } + + return createRequest({ + sourceSchema: info.schema, + sourceParentType, + sourceFieldName, + fragments: info.fragments, + variableDefinitions: info.operation.variableDefinitions, + variableValues: info.variableValues, + targetSchema, + targetOperation: operation, + targetField: fieldName, args, selectionSet, - selectionSet != null - ? undefined - : fieldNodes != null - ? fieldNodes - : info.fieldNodes, - ); + fieldNodes: + selectionSet != null + ? undefined + : fieldNodes != null + ? fieldNodes + : info.fieldNodes, + defaultArgs, + }); } -export function createRequest( - sourceSchema: GraphQLSchema, - fragments: Record, - variableDefinitions: ReadonlyArray, - variableValues: Record, - targetSchemaOrSchemaConfig: GraphQLSchema | SubschemaConfig, - targetOperation: Operation, - targetField: string, - args: Record, - selectionSet: SelectionSetNode, - fieldNodes: ReadonlyArray, -): Request { +export function createRequest({ + sourceSchema, + sourceParentType, + sourceFieldName, + fragments, + variableDefinitions, + variableValues, + targetSchema, + targetOperation, + targetField, + args, + selectionSet, + fieldNodes, + defaultArgs, +}: ICreateRequest): Request { let argumentNodes: ReadonlyArray; let newSelectionSet: SelectionSetNode = selectionSet; @@ -118,24 +145,25 @@ export function createRequest( variables[varName] = serializeInputValue(varType, variableValues[varName]); } - if (args != null) { - const { - arguments: updatedArguments, - variableDefinitions: updatedVariableDefinitions, - variableValues: updatedVariableValues, - } = updateArguments( - targetSchemaOrSchemaConfig, - targetOperation, - targetField, - argumentNodes, - variableDefinitions, - variables, - args, - ); - argumentNodes = updatedArguments; - newVariableDefinitions = updatedVariableDefinitions; - variables = updatedVariableValues; - } + const { + arguments: updatedArguments, + variableDefinitions: updatedVariableDefinitions, + variableValues: updatedVariableValues, + } = updateArguments( + sourceParentType, + sourceFieldName, + targetSchema, + targetOperation, + targetField, + argumentNodes, + variableDefinitions, + variables, + args, + defaultArgs, + ); + argumentNodes = updatedArguments; + newVariableDefinitions = updatedVariableDefinitions; + variables = updatedVariableValues; const rootfieldNode: FieldNode = { kind: Kind.FIELD, @@ -174,92 +202,129 @@ export function createRequest( } function updateArguments( - subschemaOrSubschemaConfig: GraphQLSchema | SubschemaConfig, + sourceParentType: GraphQLObjectType, + sourceFieldName: string, + targetSchema: GraphQLSchema, operation: OperationTypeNode, fieldName: string, argumentNodes: ReadonlyArray = [], variableDefinitions: ReadonlyArray = [], variableValues: Record = {}, - newArgsMap: Record = {}, + newArgs: Record = {}, + defaultArgs: Record = {}, ): { arguments: Array; variableDefinitions: Array; variableValues: Record; } { - const schema = isSubschemaConfig(subschemaOrSubschemaConfig) - ? subschemaOrSubschemaConfig.schema - : subschemaOrSubschemaConfig; - let type: GraphQLObjectType; if (operation === 'subscription') { - type = schema.getSubscriptionType(); + type = targetSchema.getSubscriptionType(); } else if (operation === 'mutation') { - type = schema.getMutationType(); + type = targetSchema.getMutationType(); } else { - type = schema.getQueryType(); + type = targetSchema.getQueryType(); } - const varNames = variableDefinitions.reduce((acc, def) => { - acc[def.variable.name.value] = true; - return acc; - }, {}); + const updatedVariableDefinitions = {}; + const varNames = {}; + variableDefinitions.forEach((def) => { + const varName = def.variable.name.value; + updatedVariableDefinitions[varName] = def; + varNames[varName] = true; + }); let numGeneratedVariables = 0; const updatedArgs: Record = {}; argumentNodes.forEach((argument: ArgumentNode) => { updatedArgs[argument.name.value] = argument; }); - const newVariableDefinitions: Array = []; const field: GraphQLField = type.getFields()[fieldName]; - field.args.forEach((argument: GraphQLArgument) => { - if (newArgsMap[argument.name]) { + if (field != null) { + field.args.forEach((argument: GraphQLArgument) => { const argName = argument.name; - let varName; - do { - varName = `_v${(numGeneratedVariables++).toString()}_${argName}`; - } while (varNames[varName]); - updatedArgs[argument.name] = { - kind: Kind.ARGUMENT, - name: { - kind: Kind.NAME, - value: argName, - }, - value: { - kind: Kind.VARIABLE, - name: { - kind: Kind.NAME, - value: varName, - }, - }, - }; - varNames[varName] = true; - newVariableDefinitions.push({ - kind: Kind.VARIABLE_DEFINITION, - variable: { - kind: Kind.VARIABLE, - name: { - kind: Kind.NAME, - value: varName, - }, - }, - type: astFromType(argument.type), - }); - variableValues[varName] = serializeInputValue( - argument.type, - newArgsMap[argName], - ); - } - }); + let newArg; + let argType; + if (newArgs[argName] != null) { + newArg = newArgs[argName]; + argType = argument.type; + } else if (updatedArgs[argName] == null && defaultArgs[argName] != null) { + newArg = defaultArgs[argName]; + const sourcefield = sourceParentType.getFields()[sourceFieldName]; + argType = sourcefield.args.find((arg) => arg.name === argName).type; + } + + if (newArg != null) { + numGeneratedVariables++; + updateArgument( + argName, + argType, + numGeneratedVariables, + varNames, + updatedArgs, + updatedVariableDefinitions, + variableValues, + newArg, + ); + } + }); + } return { arguments: Object.keys(updatedArgs).map((argName) => updatedArgs[argName]), - variableDefinitions: newVariableDefinitions, + variableDefinitions: Object.keys(updatedVariableDefinitions).map( + (varName) => updatedVariableDefinitions[varName], + ), variableValues, }; } +function updateArgument( + argName: string, + argType: GraphQLInputType, + numGeneratedVariables: number, + varNames: Record, + updatedArgs: Record, + updatedVariableDefinitions: Record, + variableValues: Record, + newArg: any, +) { + let varName; + do { + varName = `_v${numGeneratedVariables.toString()}_${argName}`; + } while (varNames[varName]); + + updatedArgs[argName] = { + kind: Kind.ARGUMENT, + name: { + kind: Kind.NAME, + value: argName, + }, + value: { + kind: Kind.VARIABLE, + name: { + kind: Kind.NAME, + value: varName, + }, + }, + }; + varNames[varName] = true; + updatedVariableDefinitions[varName] = { + kind: Kind.VARIABLE_DEFINITION, + variable: { + kind: Kind.VARIABLE, + name: { + kind: Kind.NAME, + value: varName, + }, + }, + type: astFromType(argType), + }; + variableValues[varName] = serializeInputValue(argType, newArg); +} + function astFromType(type: GraphQLType): TypeNode { if (isNonNullType(type)) { const innerType = astFromType(type.ofType); diff --git a/src/delegate/delegateToSchema.ts b/src/delegate/delegateToSchema.ts index 6dadeacd382..10e6dade824 100644 --- a/src/delegate/delegateToSchema.ts +++ b/src/delegate/delegateToSchema.ts @@ -51,6 +51,7 @@ export default function delegateToSchema( const { schema: subschemaOrSubschemaConfig, + transformedSchema, info, operation = getDelegatingOperation(info.parentType, info.schema), fieldName = info.fieldName, @@ -63,6 +64,7 @@ export default function delegateToSchema( const request = createRequestFromInfo({ info, schema: subschemaOrSubschemaConfig, + transformedSchema, operation, fieldName, args, diff --git a/src/test/testAlternateMergeSchemas.ts b/src/test/testAlternateMergeSchemas.ts index 32b809b0edc..af4fac1a9f1 100644 --- a/src/test/testAlternateMergeSchemas.ts +++ b/src/test/testAlternateMergeSchemas.ts @@ -27,6 +27,7 @@ import { FilterRootFields, FilterObjectFields, RenameInterfaceFields, + TransformRootFields, } from '../wrap/index'; import { isSpecifiedScalarType, toConfig } from '../polyfills/index'; @@ -343,12 +344,14 @@ describe('merge schemas through transforms', () => { }); describe('transform object fields', () => { - let transformedPropertySchema: GraphQLSchema; - - before(() => { - transformedPropertySchema = transformSchema(propertySchema, [ + it('should work to add a resolver', async () => { + const transformedPropertySchema = transformSchema(propertySchema, [ new TransformObjectFields( - (typeName: string, fieldName: string, field: GraphQLField) => { + ( + typeName: string, + fieldName: string, + field: GraphQLField, + ) => { if (typeName !== 'Property' || fieldName !== 'name') { return undefined; } @@ -372,9 +375,7 @@ describe('transform object fields', () => { }, ), ]); - }); - it('should work', async () => { const result = await graphql( transformedPropertySchema, ` @@ -409,6 +410,43 @@ describe('transform object fields', () => { }); }); +describe('default values', () => { + it('should work to add a default value even when renaming root fields', async () => { + const transformedPropertySchema = transformSchema(propertySchema, [ + new TransformRootFields( + ( + typeName: string, + fieldName: string, + field: GraphQLField, + ) => { + if (typeName === 'Query' && fieldName === 'jsonTest') { + const fieldConfig = toConfig(field); + fieldConfig.args.input.defaultValue = { test: 'test' }; + return { name: 'renamedJsonTest', field: fieldConfig }; + } + }, + ), + ]); + + const result = await graphql( + transformedPropertySchema, + ` + query { + renamedJsonTest + } + `, + ); + + expect(result).to.deep.equal({ + data: { + renamedJsonTest: { + test: 'test', + }, + }, + }); + }); +}); + describe('rename fields that implement interface fields', () => { it('should work', () => { const originalItem = { diff --git a/src/test/testDelegateToSchema.ts b/src/test/testDelegateToSchema.ts index 3119ed5b5e3..c94718fdf72 100644 --- a/src/test/testDelegateToSchema.ts +++ b/src/test/testDelegateToSchema.ts @@ -4,6 +4,8 @@ import { expect } from 'chai'; import delegateToSchema from '../delegate/delegateToSchema'; import mergeSchemas from '../stitch/mergeSchemas'; import { IResolvers } from '../Interfaces'; +import { makeExecutableSchema } from '../generate'; +import { wrapSchema } from '../wrap'; import { propertySchema, @@ -115,3 +117,97 @@ describe('stitching', () => { }); }); }); + +describe('schema delegation', () => { + it('should work even when there are default fields', async () => { + const schema = makeExecutableSchema({ + typeDefs: ` + scalar JSON + type Data { + json(input: JSON = "test"): JSON + } + type Query { + data: Data + } + `, + resolvers: { + Query: { + data: () => ({}), + }, + Data: { + json: (_root, args, context, info) => + delegateToSchema({ + schema: propertySchema, + fieldName: 'jsonTest', + args, + context, + info, + }), + }, + }, + }); + + const result = await graphql( + schema, + ` + query { + data { + json + } + } + `, + ); + + expect(result).to.deep.equal({ + data: { + data: { + json: 'test', + }, + }, + }); + }); + + it('should work even with variables', async () => { + const innerSchema = makeExecutableSchema({ + typeDefs: ` + type User { + id(show: Boolean): ID + } + type Query { + user: User + } + `, + resolvers: { + Query: { + user: () => ({}), + }, + User: { + id: () => '123', + }, + }, + }); + const schema = wrapSchema(innerSchema); + + const result = await graphql( + schema, + ` + query($show: Boolean) { + user { + id(show: $show) + } + } + `, + null, + null, + { show: true }, + ); + + expect(result).to.deep.equal({ + data: { + user: { + id: '123', + }, + }, + }); + }); +}); diff --git a/src/wrap/index.ts b/src/wrap/index.ts index 077f7f1a12e..6945c5450bd 100644 --- a/src/wrap/index.ts +++ b/src/wrap/index.ts @@ -317,7 +317,8 @@ export { applyResultTransforms, } from './transforms'; -export { default as transformSchema, wrapSchema } from './transformSchema'; +export { transformSchema } from './transformSchema'; +export { wrapSchema } from './wrapSchema'; export * from './transforms/index'; diff --git a/src/wrap/resolvers.ts b/src/wrap/resolvers.ts index 9967bafc3a8..dc9662ba1f7 100644 --- a/src/wrap/resolvers.ts +++ b/src/wrap/resolvers.ts @@ -17,6 +17,8 @@ import { makeMergedType } from '../stitch/makeMergedType'; import { getResponseKeyFromInfo } from '../stitch/getResponseKeyFromInfo'; import { getErrors, getSubschema } from '../stitch/proxiedResult'; +import { applySchemaTransforms } from './transforms'; + export type Mapping = { [typeName: string]: { [fieldName: string]: { @@ -119,6 +121,19 @@ function defaultCreateProxyingResolver({ schema: SubschemaConfig; transforms: Array; }): GraphQLFieldResolver { + let schemaTransforms: Array = []; + if (schema.transforms != null) { + schemaTransforms = schemaTransforms.concat(schema.transforms); + } + if (transforms != null) { + schemaTransforms = schemaTransforms.concat(transforms); + } + + const transformedSchema = applySchemaTransforms( + schema.schema, + schemaTransforms, + ); + return (parent, _args, context, info) => { if (parent != null) { const responseKey = getResponseKeyFromInfo(info); @@ -140,6 +155,7 @@ function defaultCreateProxyingResolver({ context, info, transforms, + transformedSchema, }); }; } diff --git a/src/wrap/transformSchema.ts b/src/wrap/transformSchema.ts index 51c9c962d4e..61481aa0801 100644 --- a/src/wrap/transformSchema.ts +++ b/src/wrap/transformSchema.ts @@ -1,50 +1,18 @@ import { GraphQLSchema } from 'graphql'; -import { addResolversToSchema } from '../generate/index'; import { Transform, SubschemaConfig, - isSubschemaConfig, GraphQLSchemaWithTransforms, } from '../Interfaces'; -import { cloneSchema } from '../utils/index'; -import { generateProxyingResolvers, stripResolvers } from './resolvers'; -import { applySchemaTransforms } from './transforms'; +import { wrapSchema } from './wrapSchema'; -export function wrapSchema( - subschemaOrSubschemaConfig: GraphQLSchema | SubschemaConfig, - transforms?: Array, -): GraphQLSchema { - const subschemaConfig: SubschemaConfig = isSubschemaConfig( - subschemaOrSubschemaConfig, - ) - ? subschemaOrSubschemaConfig - : { schema: subschemaOrSubschemaConfig }; - - const schema = cloneSchema(subschemaConfig.schema); - stripResolvers(schema); - - addResolversToSchema({ - schema, - resolvers: generateProxyingResolvers({ subschemaConfig, transforms }), - resolverValidationOptions: { - allowResolversNotInSchema: true, - }, - }); - - let schemaTransforms: Array = []; - if (subschemaConfig.transforms != null) { - schemaTransforms = schemaTransforms.concat(subschemaConfig.transforms); - } - if (transforms != null) { - schemaTransforms = schemaTransforms.concat(transforms); - } - - return applySchemaTransforms(schema, schemaTransforms); -} - -export default function transformSchema( +// This function is deprecated in favor of wrapSchema as the name is misleading. +// transformSchema does not just "transform" a schema, it wraps a schema with transforms +// using a round of delegation. +// The applySchemaTransforms function actually "transforms" the schema and is used during wrapping. +export function transformSchema( subschemaOrSubschemaConfig: GraphQLSchema | SubschemaConfig, transforms: Array, ): GraphQLSchemaWithTransforms { diff --git a/src/wrap/wrapSchema.ts b/src/wrap/wrapSchema.ts new file mode 100644 index 00000000000..a60bb19c0b6 --- /dev/null +++ b/src/wrap/wrapSchema.ts @@ -0,0 +1,40 @@ +import { GraphQLSchema } from 'graphql'; + +import { addResolversToSchema } from '../generate/index'; +import { Transform, SubschemaConfig, isSubschemaConfig } from '../Interfaces'; +import { cloneSchema } from '../utils/index'; + +import { generateProxyingResolvers, stripResolvers } from './resolvers'; +import { applySchemaTransforms } from './transforms'; + +export function wrapSchema( + subschemaOrSubschemaConfig: GraphQLSchema | SubschemaConfig, + transforms?: Array, +): GraphQLSchema { + const subschemaConfig: SubschemaConfig = isSubschemaConfig( + subschemaOrSubschemaConfig, + ) + ? subschemaOrSubschemaConfig + : { schema: subschemaOrSubschemaConfig }; + + const schema = cloneSchema(subschemaConfig.schema); + stripResolvers(schema); + + addResolversToSchema({ + schema, + resolvers: generateProxyingResolvers({ subschemaConfig, transforms }), + resolverValidationOptions: { + allowResolversNotInSchema: true, + }, + }); + + let schemaTransforms: Array = []; + if (subschemaConfig.transforms != null) { + schemaTransforms = schemaTransforms.concat(subschemaConfig.transforms); + } + if (transforms != null) { + schemaTransforms = schemaTransforms.concat(transforms); + } + + return applySchemaTransforms(schema, schemaTransforms); +}