From 9dbcdeae56870eaad4ba09c6ec9c040c65c67cdd Mon Sep 17 00:00:00 2001 From: AndresPrez Date: Sun, 29 May 2022 09:30:36 -0400 Subject: [PATCH] feat: Extend FieldArgTypeCoercion (#50) * enhances for argtype coercion * Adds unit tests for new functionalities * fixes linting issues --- src/RewriteHandler.ts | 2 +- src/rewriters/FieldArgTypeRewriter.ts | 133 ++++++++++++++++--- src/rewriters/Rewriter.ts | 22 ++- test/functional/rewriteFieldArgType.test.ts | 140 ++++++++++++++++++++ 4 files changed, 270 insertions(+), 27 deletions(-) diff --git a/src/RewriteHandler.ts b/src/RewriteHandler.ts index 505448d..92b3dd2 100644 --- a/src/RewriteHandler.ts +++ b/src/RewriteHandler.ts @@ -39,7 +39,7 @@ export default class RewriteHandler { const isMatch = rewriter.matches(nodeAndVars, parents); if (isMatch) { rewrittenVariables = rewriter.rewriteVariables(rewrittenNodeAndVars, rewrittenVariables); - rewrittenNodeAndVars = rewriter.rewriteQuery(rewrittenNodeAndVars); + rewrittenNodeAndVars = rewriter.rewriteQuery(rewrittenNodeAndVars, rewrittenVariables); const simplePath = extractPath([...parents, rewrittenNodeAndVars.node]); let paths: ReadonlyArray> = [simplePath]; const fragmentDef = parents.find(({ kind }) => kind === 'FragmentDefinition') as diff --git a/src/rewriters/FieldArgTypeRewriter.ts b/src/rewriters/FieldArgTypeRewriter.ts index 294c0ec..dbac77a 100644 --- a/src/rewriters/FieldArgTypeRewriter.ts +++ b/src/rewriters/FieldArgTypeRewriter.ts @@ -1,4 +1,15 @@ -import { ArgumentNode, ASTNode, FieldNode, parseType, TypeNode, VariableNode } from 'graphql'; +import { + ArgumentNode, + ASTNode, + FieldNode, + isValueNode, + Kind, + parseType, + TypeNode, + ValueNode, + VariableNode +} from 'graphql'; +import Maybe from 'graphql/tsutils/Maybe'; import { NodeAndVarDefs, nodesMatch } from '../ast'; import { identifyFunc } from '../utils'; import Rewriter, { RewriterOpts, Variables } from './Rewriter'; @@ -7,7 +18,17 @@ interface FieldArgTypeRewriterOpts extends RewriterOpts { argName: string; oldType: string; newType: string; - coerceVariable?: (variable: any) => any; + coerceVariable?: (variable: any, context: { variables: Variables; args: ArgumentNode[] }) => any; + /** + * EXPERIMENTAL: + * This allows to coerce value of argument when their value is not stored in a variable + * but comes in the query node itself. + * NOTE: At the moment, the user has to return the ast value node herself. + */ + coerceArgumentValue?: ( + variable: any, + context: { variables: Variables; args: ArgumentNode[] } + ) => Maybe; } /** @@ -18,7 +39,19 @@ class FieldArgTypeRewriter extends Rewriter { protected argName: string; protected oldTypeNode: TypeNode; protected newTypeNode: TypeNode; - protected coerceVariable: (variable: any) => any; + // Passes context with rest of arguments and variables. + // Quite useful for variable coercion that depends on other arguments/variables + // (e.g., [offset, limit] to [pageSize, pageNumber] coercion) + protected coerceVariable: ( + variable: any, + context: { variables: Variables; args: ArgumentNode[] } + ) => any; + // (Experimental): Used to coerce arguments whose value + // does not come in a variable. + protected coerceArgumentValue: ( + variable: any, + context: { variables: Variables; args: ArgumentNode[] } + ) => Maybe; constructor(options: FieldArgTypeRewriterOpts) { super(options); @@ -26,6 +59,7 @@ class FieldArgTypeRewriter extends Rewriter { this.oldTypeNode = parseType(options.oldType); this.newTypeNode = parseType(options.newType); this.coerceVariable = options.coerceVariable || identifyFunc; + this.coerceArgumentValue = options.coerceArgumentValue || identifyFunc; } public matches(nodeAndVars: NodeAndVarDefs, parents: ASTNode[]) { @@ -34,39 +68,94 @@ class FieldArgTypeRewriter extends Rewriter { const { variableDefinitions } = nodeAndVars; // is this a field with the correct fieldName and arguments? if (node.kind !== 'Field') return false; - if (node.name.value !== this.fieldName || !node.arguments) return false; + + // does this field contain arguments? + if (!node.arguments) return false; + // is there an argument with the correct name and type in a variable? const matchingArgument = node.arguments.find(arg => arg.name.value === this.argName); - if (!matchingArgument || matchingArgument.value.kind !== 'Variable') return false; - const varRef = matchingArgument.value.name.value; - // does the referenced variable have the correct type? - for (const varDefinition of variableDefinitions) { - if (varDefinition.variable.name.value === varRef) { - return nodesMatch(this.oldTypeNode, varDefinition.type); + if (!matchingArgument) return false; + + // argument value is stored in a variable + if (matchingArgument.value.kind === 'Variable') { + const varRef = matchingArgument.value.name.value; + // does the referenced variable have the correct type? + for (const varDefinition of variableDefinitions) { + if (varDefinition.variable.name.value === varRef) { + return nodesMatch(this.oldTypeNode, varDefinition.type); + } } } + // argument value comes in query doc. + else { + const argValueNode = matchingArgument.value; + return isValueNode(argValueNode); + // Would be ideal to do a nodesMatch in here, however argument value nodes + // have different format for their values than when passed as variables. + // For instance, are parsed with Kinds as "graphql.Kind" (e.g., INT="IntValue") and not "graphql.TokenKinds" (e.g., INT="Int") + // So they might not match correctly. Also they dont contain additional parsed syntax + // as the non-optional symbol "!". So just return true if the argument.value is a ValueNode. + // + // return nodesMatch(this.oldTypeNode, parseType(argRef.kind)); + } + return false; } - public rewriteQuery({ node, variableDefinitions }: NodeAndVarDefs) { - const varRefName = this.extractMatchingVarRefName(node as FieldNode); - const newVarDefs = variableDefinitions.map(varDef => { - if (varDef.variable.name.value !== varRefName) return varDef; - return { ...varDef, type: this.newTypeNode }; - }); - return { node, variableDefinitions: newVarDefs }; + public rewriteQuery( + { node: astNode, variableDefinitions }: NodeAndVarDefs, + variables: Variables + ) { + const node = astNode as FieldNode; + const varRefName = this.extractMatchingVarRefName(node); + // If argument value is stored in a variable + if (varRefName) { + const newVarDefs = variableDefinitions.map(varDef => { + if (varDef.variable.name.value !== varRefName) return varDef; + return { ...varDef, type: this.newTypeNode }; + }); + return { node, variableDefinitions: newVarDefs }; + } + // If argument value is not stored in a variable but in the query node. + const matchingArgument = (node.arguments || []).find(arg => arg.name.value === this.argName); + if (node.arguments && matchingArgument) { + const args = [...node.arguments]; + const newValue = this.coerceArgumentValue(matchingArgument.value, { variables, args }); + /** + * TODO: If somewhow we can get the schema here, we could make the coerceArgumentValue + * even easier, as we would be able to construct the ast node for the argument value. + * as of now, the user has to take care of correctly constructing the argument value ast node herself. + * + * const schema = makeExecutableSchema({typeDefs}) + * const myCustomType = schema.getType("MY_CUSTOM_TYPE_NAME") + * const newArgValue = astFromValue(newValue, myCustomType) + * Object.assign(matchingArgument, { value: newArgValue }) + */ + if (newValue) Object.assign(matchingArgument, { value: newValue }); + } + return { node, variableDefinitions }; } - public rewriteVariables({ node }: NodeAndVarDefs, variables: Variables) { + public rewriteVariables({ node: astNode }: NodeAndVarDefs, variables: Variables) { + const node = astNode as FieldNode; if (!variables) return variables; - const varRefName = this.extractMatchingVarRefName(node as FieldNode); - return { ...variables, [varRefName]: this.coerceVariable(variables[varRefName]) }; + const varRefName = this.extractMatchingVarRefName(node); + const args = [...(node.arguments ? node.arguments : [])]; + return { + ...variables, + ...(varRefName + ? { [varRefName]: this.coerceVariable(variables[varRefName], { variables, args }) } + : {}) + }; } private extractMatchingVarRefName(node: FieldNode) { - const matchingArgument = (node.arguments || []).find(arg => arg.name.value === this.argName); - return ((matchingArgument as ArgumentNode).value as VariableNode).name.value; + const matchingArgument = (node.arguments || []).find( + arg => arg.name.value === this.argName + ) as ArgumentNode; + const variableNode = matchingArgument.value as VariableNode; + return variableNode.kind === Kind.VARIABLE && variableNode.name.value; } } diff --git a/src/rewriters/Rewriter.ts b/src/rewriters/Rewriter.ts index f33d659..f138596 100644 --- a/src/rewriters/Rewriter.ts +++ b/src/rewriters/Rewriter.ts @@ -6,7 +6,7 @@ export type Variables = { [key: string]: any } | undefined; export type RootType = 'query' | 'mutation' | 'fragment'; export interface RewriterOpts { - fieldName: string; + fieldName?: string; rootTypes?: RootType[]; matchConditions?: matchCondition[]; } @@ -16,19 +16,33 @@ export interface RewriterOpts { * Extend this class and overwrite its methods to create a new rewriter */ abstract class Rewriter { - protected fieldName: string; protected rootTypes: RootType[] = ['query', 'mutation', 'fragment']; + protected fieldName?: string; protected matchConditions?: matchCondition[]; constructor({ fieldName, rootTypes, matchConditions }: RewriterOpts) { this.fieldName = fieldName; this.matchConditions = matchConditions; + if (!this.fieldName && !this.matchConditions) { + throw new Error( + 'Neither a fieldName or matchConditions were provided. Please choose to pass either one in order to be able to detect which fields to rewrite.' + ); + } if (rootTypes) this.rootTypes = rootTypes; } public matches(nodeAndVarDefs: NodeAndVarDefs, parents: ReadonlyArray): boolean { const { node } = nodeAndVarDefs; - if (node.kind !== 'Field' || node.name.value !== this.fieldName) return false; + + // If no fieldName is provided, check for defined matchConditions. + // This avoids having to define one rewriter for many fields individually. + // Alternatively, regex matching for fieldName could be implemented. + if ( + node.kind !== 'Field' || + (this.fieldName ? node.name.value !== this.fieldName : !this.matchConditions) + ) { + return false; + } const root = parents[0]; if ( root.kind === 'OperationDefinition' && @@ -48,7 +62,7 @@ abstract class Rewriter { return true; } - public rewriteQuery(nodeAndVarDefs: NodeAndVarDefs): NodeAndVarDefs { + public rewriteQuery(nodeAndVarDefs: NodeAndVarDefs, variables: Variables): NodeAndVarDefs { return nodeAndVarDefs; } diff --git a/test/functional/rewriteFieldArgType.test.ts b/test/functional/rewriteFieldArgType.test.ts index 9f8d173..af47e1c 100644 --- a/test/functional/rewriteFieldArgType.test.ts +++ b/test/functional/rewriteFieldArgType.test.ts @@ -1,3 +1,4 @@ +import { FieldNode, astFromValue, GraphQLInt, Kind } from 'graphql'; import RewriteHandler from '../../src/RewriteHandler'; import FieldArgTypeRewriter from '../../src/rewriters/FieldArgTypeRewriter'; import { gqlFmt } from '../testUtils'; @@ -98,6 +99,145 @@ describe('Rewrite field arg type', () => { }); }); + it('variable coercion comes with additional variables and arguments as context.', () => { + const query = gqlFmt` + query doTheThings($arg1: String!, $arg2: String) { + things(identifier: $arg1, arg2: $arg2, arg3: "blah") { + cat + } + } + `; + const expectedRewritenQuery = gqlFmt` + query doTheThings($arg1: Int!, $arg2: String) { + things(identifier: $arg1, arg2: $arg2, arg3: "blah") { + cat + } + } + `; + + const handler = new RewriteHandler([ + new FieldArgTypeRewriter({ + fieldName: 'things', + argName: 'identifier', + oldType: 'String!', + newType: 'Int!', + coerceVariable: (_, { variables = {}, args }) => { + expect(args.length).toBe(3); + expect(args[0].kind).toBe('Argument'); + expect(args[0].value.kind).toBe(Kind.VARIABLE); + expect(args[1].kind).toBe('Argument'); + expect(args[1].value.kind).toBe(Kind.VARIABLE); + expect(args[2].kind).toBe('Argument'); + expect(args[2].value.kind).toBe(Kind.STRING); + const { arg2 = 0 } = variables; + return parseInt(arg2, 10); + } + }) + ]); + expect(handler.rewriteRequest(query, { arg1: 'someString', arg2: '123' })).toEqual({ + query: expectedRewritenQuery, + variables: { + arg1: 123, + arg2: '123' + } + }); + }); + + it('can be passed a coerceArgumentValue function to change argument values.', () => { + const query = gqlFmt` + query doTheThings { + things(identifier: "123", arg2: "blah") { + cat + } + } + `; + const expectedRewritenQuery = gqlFmt` + query doTheThings { + things(identifier: 123, arg2: "blah") { + cat + } + } + `; + + const handler = new RewriteHandler([ + new FieldArgTypeRewriter({ + fieldName: 'things', + argName: 'identifier', + oldType: 'String!', + newType: 'Int!', + coerceArgumentValue: argValue => { + const value = argValue.value; + const newArgValue = astFromValue(parseInt(value, 10), GraphQLInt); + return newArgValue; + } + }) + ]); + + expect(handler.rewriteRequest(query)).toEqual({ + query: expectedRewritenQuery + }); + }); + + it('should fail if neither a fieldName or matchConditions are provided', () => { + try { + new FieldArgTypeRewriter({ + argName: 'identifier', + oldType: 'String!', + newType: 'Int!' + }); + } catch (error) { + console.log(error.message); + expect( + error.message.includes('Neither a fieldName or matchConditions were provided') + ).toEqual(true); + } + }); + + it('allows matching using matchConditions when fieldName is not provided.', () => { + const query = gqlFmt` + query doTheThings($arg1: String!, $arg2: String) { + things(identifier: $arg1, arg2: $arg2) { + cat + } + } + `; + const expectedRewritenQuery = gqlFmt` + query doTheThings($arg1: Int!, $arg2: String) { + things(identifier: $arg1, arg2: $arg2) { + cat + } + } + `; + + // Tests a dummy regex to match the "things" field. + const fieldNameRegExp = '.hings'; + + const handler = new RewriteHandler([ + new FieldArgTypeRewriter({ + argName: 'identifier', + oldType: 'String!', + newType: 'Int!', + matchConditions: [ + nodeAndVars => { + const node = nodeAndVars.node as FieldNode; + const { + name: { value: fieldName } + } = node; + return fieldName.search(new RegExp(fieldNameRegExp)) !== -1; + } + ], + coerceVariable: val => parseInt(val, 10) + }) + ]); + expect(handler.rewriteRequest(query, { arg1: '123', arg2: 'blah' })).toEqual({ + query: expectedRewritenQuery, + variables: { + arg1: 123, + arg2: 'blah' + } + }); + }); + it('works on deeply nested fields', () => { const query = gqlFmt` query doTheThings($arg1: String!, $arg2: String) {