Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extends FieldArgTypeCoercion #50

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/RewriteHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ReadonlyArray<string>> = [simplePath];
const fragmentDef = parents.find(({ kind }) => kind === 'FragmentDefinition') as
Expand Down
133 changes: 111 additions & 22 deletions src/rewriters/FieldArgTypeRewriter.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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<ValueNode>;
}

/**
Expand All @@ -18,14 +39,27 @@ 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<ValueNode>;

constructor(options: FieldArgTypeRewriterOpts) {
super(options);
this.argName = options.argName;
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[]) {
Expand All @@ -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;
}
}

Expand Down
22 changes: 18 additions & 4 deletions src/rewriters/Rewriter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}
Expand All @@ -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<ASTNode>): 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 this is a cool idea for a future PR!

if (
node.kind !== 'Field' ||
(this.fieldName ? node.name.value !== this.fieldName : !this.matchConditions)
) {
return false;
}
const root = parents[0];
if (
root.kind === 'OperationDefinition' &&
Expand All @@ -48,7 +62,7 @@ abstract class Rewriter {
return true;
}

public rewriteQuery(nodeAndVarDefs: NodeAndVarDefs): NodeAndVarDefs {
public rewriteQuery(nodeAndVarDefs: NodeAndVarDefs, variables: Variables): NodeAndVarDefs {
return nodeAndVarDefs;
}

Expand Down
140 changes: 140 additions & 0 deletions test/functional/rewriteFieldArgType.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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) {
Expand Down