Skip to content

Commit a0ef8b7

Browse files
committed
updated typeComplexity tests to better account for GraphQL variables.
Identified bugs when parsing fieldNodes and building typeWeight objects
1 parent a88aa69 commit a0ef8b7

File tree

5 files changed

+58
-26
lines changed

5 files changed

+58
-26
lines changed

src/@types/buildTypeWeights.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,7 @@ export interface TypeWeightConfig {
1717
scalar?: number;
1818
connection?: number;
1919
}
20+
21+
type Variables = {
22+
[index: string]: readonly unknown;
23+
};

src/analysis/ASTnodefunctions.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
SelectionNode,
99
ArgumentNode,
1010
} from 'graphql';
11-
import { FieldWeight, TypeWeightObject } from '../@types/buildTypeWeights';
11+
import { FieldWeight, TypeWeightObject, Variables } from '../@types/buildTypeWeights';
1212

1313
// TODO: handle variables and arguments
1414
// ! this is not functional
@@ -48,7 +48,7 @@ const getArgObj = (args: ArgumentNode[]): { [index: string]: any } => {
4848
export function fieldNode(
4949
node: FieldNode,
5050
typeWeights: TypeWeightObject,
51-
variables: any | undefined,
51+
variables: Variables,
5252
parentName: string
5353
): number {
5454
let complexity = 0;
@@ -73,6 +73,8 @@ export function fieldNode(
7373
// if the feild weight is a number, add the number to the total complexity
7474
complexity += fieldWeight;
7575
} else if (node.arguments) {
76+
// BUG: This code is reached when fieldWeight is undefined, which could result from an invalid query or this type
77+
// missing from the typeWeight object. If left unhandled an error is thrown
7678
// otherwise the the feild weight is a list, invoke the function with variables
7779
// TODO: calculate the complexity for lists with arguments and varibales
7880
// ! this is not functional
@@ -86,7 +88,7 @@ export function fieldNode(
8688
export function selectionNode(
8789
node: SelectionNode,
8890
typeWeights: TypeWeightObject,
89-
variables: any | undefined,
91+
variables: Variables,
9092
parentName: string
9193
): number {
9294
let complexity = 0;
@@ -103,7 +105,7 @@ export function selectionNode(
103105
export function selectionSetNode(
104106
node: SelectionSetNode,
105107
typeWeights: TypeWeightObject,
106-
variables: any | undefined,
108+
variables: Variables,
107109
parentName: string
108110
): number {
109111
let complexity = 0;
@@ -119,7 +121,7 @@ export function selectionSetNode(
119121
export function definitionNode(
120122
node: DefinitionNode,
121123
typeWeights: TypeWeightObject,
122-
variables: any | undefined
124+
variables: Variables
123125
): number {
124126
let complexity = 0;
125127
// check the kind property against the set of definiton nodes that are possible
@@ -145,7 +147,7 @@ export function definitionNode(
145147
export function documentNode(
146148
node: DocumentNode,
147149
typeWeights: TypeWeightObject,
148-
variables: any | undefined
150+
variables: Variables
149151
): number {
150152
let complexity = 0;
151153
// iterate through 'definitions' array on the document node

src/analysis/buildTypeWeights.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ function parseQuery(
8383
// If query has an argument matching one of the limiting keywords and resolves to a list then the weight of the query
8484
// should be dependent on both the weight of the resolved type and the limiting argument.
8585
// FIXME: Can nonnull wrap list types?
86+
// BUG: Lists need to be accounted for in all types not just queries
8687
if (KEYWORDS.includes(arg.name) && isListType(resolveType)) {
8788
// Get the type that comprises the list
8889
const listType = resolveType.ofType;

src/analysis/typeComplexityAnalysis.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { DocumentNode } from 'graphql';
2-
import { TypeWeightObject } from '../@types/buildTypeWeights';
2+
import { TypeWeightObject, Variables } from '../@types/buildTypeWeights';
33
import { documentNode } from './ASTnodefunctions';
44

55
/**
@@ -9,12 +9,12 @@ import { documentNode } from './ASTnodefunctions';
99
* TO DO: extend the functionality to work for mutations and subscriptions and directives
1010
*
1111
* @param {string} queryAST
12-
* @param {any | undefined} varibales
12+
* @param {Variables} variables
1313
* @param {TypeWeightObject} typeWeights
1414
*/
1515
function getQueryTypeComplexity(
1616
queryAST: DocumentNode,
17-
variables: any | undefined,
17+
variables: Variables,
1818
typeWeights: TypeWeightObject
1919
): number {
2020
let complexity = 0;

test/analysis/typeComplexityAnalysis.test.ts

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
import { ArgumentNode } from 'graphql/language';
21
import { parse } from 'graphql';
32
import getQueryTypeComplexity from '../../src/analysis/typeComplexityAnalysis';
4-
import { TypeWeightObject } from '../../src/@types/buildTypeWeights';
3+
import { TypeWeightObject, Variables } from '../../src/@types/buildTypeWeights';
54

65
/**
76
* Here is the schema that creates the followning 'typeWeightsObject' used for the tests
@@ -97,15 +96,23 @@ import { TypeWeightObject } from '../../src/@types/buildTypeWeights';
9796
to character, human and droid
9897
*/
9998

99+
const mockWeightFunction = jest.fn();
100+
const mockHumanFriendsFunction = jest.fn();
101+
const mockDroidFriendsFunction = jest.fn();
102+
100103
// this object is created by the schema above for use in all the tests below
101104
const typeWeights: TypeWeightObject = {
102105
query: {
103106
// object type
104107
weight: 1,
105108
fields: {
106-
// FIXME: update the function def that is supposed te be here to match implementation
107-
// FIXME: add the function definition for the 'search' field which returns a list
108-
reviews: (args: ArgumentNode[]): number => 10,
109+
reviews: mockWeightFunction,
110+
hero: 1,
111+
search: jest.fn(), // FIXME: Unbounded list result
112+
character: 1,
113+
droid: 1,
114+
human: 1,
115+
scalars: 1,
109116
},
110117
},
111118
episode: {
@@ -129,6 +136,8 @@ const typeWeights: TypeWeightObject = {
129136
id: 0,
130137
name: 0,
131138
homePlanet: 0,
139+
appearsIn: jest.fn(), // FIXME: resolves to an unbounded list of enums. We aren't handling this yet
140+
friends: mockHumanFriendsFunction,
132141
},
133142
},
134143
droid: {
@@ -137,6 +146,8 @@ const typeWeights: TypeWeightObject = {
137146
fields: {
138147
id: 0,
139148
name: 0,
149+
appearsIn: jest.fn(), // FIXME: resolves to an unbounded list of enums. We aren't handling this yet
150+
friends: mockDroidFriendsFunction,
140151
},
141152
},
142153
review: {
@@ -170,8 +181,13 @@ const typeWeights: TypeWeightObject = {
170181
};
171182

172183
describe('Test getQueryTypeComplexity function', () => {
184+
afterEach(() => {
185+
jest.clearAllMocks();
186+
});
187+
173188
let query = '';
174-
let variables: any | undefined;
189+
let variables: Variables = {};
190+
175191
describe('Calculates the correct type complexity for queries', () => {
176192
test('with one feild', () => {
177193
query = `query { scalars { num } }`;
@@ -203,14 +219,14 @@ describe('Test getQueryTypeComplexity function', () => {
203219
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(2); // Query 1 + scalar 1
204220
});
205221

206-
xtest('with arguments and variables', () => {
222+
test('with arguments and variables', () => {
207223
query = `query { hero(episode: EMPIRE) { id, name } }`;
208224
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(2); // Query 1 + hero/character 1
209-
query = `query { human(id: 1) { id, name, appearsIn } }`;
210-
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(3); // Query 1 + human/character 1 + appearsIn/episode
225+
query = `query { human(id: 1) { id, name, homePlanet } }`;
226+
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(2); // Query 1 + human/character 1
211227
// argument passed in as a variable
212228
variables = { ep: 'EMPIRE' };
213-
query = `query varibaleQuery ($ep: Episode){ hero(episode: $ep) { id, name } }`;
229+
query = `query variableQuery ($ep: Episode){ hero(episode: $ep) { id, name } }`;
214230
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(2); // Query 1 + hero/character 1
215231
});
216232

@@ -263,15 +279,22 @@ describe('Test getQueryTypeComplexity function', () => {
263279
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(false); // ?
264280
});
265281

266-
xtest('with lists detrmined by arguments and variables', () => {
267-
query = `Query {reviews(episode: EMPIRE, first: 3) { stars, commentary } }`;
268-
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(4); // 1 Query + 3 reviews
269-
variables = { first: 3 };
270-
query = `Query queryVaribales($first: Int) {reviews(episode: EMPIRE, first: $first) { stars, commentary } }`;
271-
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(4); // 1 Query + 3 reviews
282+
test('with lists determined by arguments and variables', () => {
283+
query = `query {reviews(episode: EMPIRE, first: 3) { stars, commentary } }`;
284+
mockWeightFunction.mockReturnValueOnce(3);
285+
expect(getQueryTypeComplexity(parse(query), {}, typeWeights)).toBe(4); // 1 Query + 3 reviews
286+
expect(mockWeightFunction.mock.calls.length).toBe(1);
287+
expect(mockWeightFunction.mock.calls[0].length).toBe(1);
288+
289+
variables = { first: 4 };
290+
mockWeightFunction.mockReturnValueOnce(4);
291+
query = `query queryVariables($first: Int) {reviews(episode: EMPIRE, first: $first) { stars, commentary } }`;
292+
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(5); // 1 Query + 3 reviews
293+
expect(mockWeightFunction.mock.calls.length).toBe(2);
294+
expect(mockWeightFunction.mock.calls[1].length).toBe(1);
272295
});
273296

274-
xtest('with nested lists', () => {
297+
test('with nested lists', () => {
275298
query = `
276299
query {
277300
human(id: 1) {
@@ -284,7 +307,9 @@ describe('Test getQueryTypeComplexity function', () => {
284307
}
285308
}
286309
}`;
310+
mockHumanFriendsFunction.mockReturnValueOnce(5).mockReturnValueOnce(3);
287311
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(17); // 1 Query + 1 human/character + (5 friends/character X 3 friends/characters)
312+
expect(mockHumanFriendsFunction.mock.calls.length).toBe(2);
288313
});
289314

290315
xtest('accounting for __typename feild', () => {

0 commit comments

Comments
 (0)