Skip to content

Commit 4602833

Browse files
committed
discovered bug that calculated query complxity wrong when object weight was set to zero
1 parent 4c36575 commit 4602833

File tree

5 files changed

+41
-20
lines changed

5 files changed

+41
-20
lines changed

src/@types/buildTypeWeights.d.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ export interface TypeWeightConfig {
2121
scalar?: number;
2222
connection?: number;
2323
}
24+
export interface TypeWeightSet {
25+
mutation: number;
26+
query: number;
27+
object: number;
28+
scalar: number;
29+
connection: number;
30+
}
2431
type Variables = {
2532
[index: string]: readonly unknown;
2633
};

src/analysis/buildTypeWeights.ts

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -23,30 +23,27 @@ import { ObjMap } from 'graphql/jsutils/ObjMap';
2323
import { GraphQLSchema } from 'graphql/type/schema';
2424
import {
2525
TypeWeightConfig,
26+
TypeWeightSet,
2627
TypeWeightObject,
2728
Variables,
2829
Type,
29-
Field,
3030
} from '../@types/buildTypeWeights';
3131

3232
export const KEYWORDS = ['first', 'last', 'limit'];
33-
type ListType =
34-
| GraphQLScalarType<unknown, unknown>
35-
| GraphQLObjectType<any, any>
36-
| GraphQLList<GraphQLOutputType>
37-
| GraphQLOutputType;
33+
3834
// These variables exist to provide a default value for typescript when accessing a weight
3935
// since all props are optioal in TypeWeightConfig
4036
const DEFAULT_MUTATION_WEIGHT = 10;
4137
const DEFAULT_OBJECT_WEIGHT = 1;
4238
const DEFAULT_SCALAR_WEIGHT = 0;
4339
const DEFAULT_CONNECTION_WEIGHT = 2;
4440
const DEFAULT_QUERY_WEIGHT = 1;
45-
export const defaultTypeWeightsConfig: TypeWeightConfig = {
41+
export const defaultTypeWeightsConfig: TypeWeightSet = {
4642
mutation: DEFAULT_MUTATION_WEIGHT,
4743
object: DEFAULT_OBJECT_WEIGHT,
4844
scalar: DEFAULT_SCALAR_WEIGHT,
4945
connection: DEFAULT_CONNECTION_WEIGHT,
46+
query: DEFAULT_QUERY_WEIGHT,
5047
};
5148

5249
// FIXME: What about Interface defaults
@@ -56,24 +53,24 @@ export const defaultTypeWeightsConfig: TypeWeightConfig = {
5653
*
5754
* @param {(GraphQLObjectType | GraphQLInterfaceType)} type
5855
* @param {TypeWeightObject} typeWeightObject
59-
* @param {TypeWeightConfig} typeWeights
56+
* @param {TypeWeightSet} typeWeights
6057
* @return {*} {Type}
6158
*/
6259
function parseObjectFields(
6360
type: GraphQLObjectType | GraphQLInterfaceType,
6461
typeWeightObject: TypeWeightObject,
65-
typeWeights: TypeWeightConfig
62+
typeWeights: TypeWeightSet
6663
): Type {
6764
let result: Type;
6865
switch (type.name) {
6966
case 'Query':
70-
result = { weight: typeWeights.query || DEFAULT_QUERY_WEIGHT, fields: {} };
67+
result = { weight: typeWeights.query, fields: {} };
7168
break;
7269
case 'Mutation':
73-
result = { weight: typeWeights.mutation || DEFAULT_MUTATION_WEIGHT, fields: {} };
70+
result = { weight: typeWeights.mutation, fields: {} };
7471
break;
7572
default:
76-
result = { weight: typeWeights.object || DEFAULT_OBJECT_WEIGHT, fields: {} };
73+
result = { weight: typeWeights.object, fields: {} };
7774
break;
7875
}
7976

@@ -88,7 +85,7 @@ function parseObjectFields(
8885
(isNonNullType(fieldType) && isScalarType(fieldType.ofType))
8986
) {
9087
result.fields[field] = {
91-
weight: typeWeights.scalar || DEFAULT_SCALAR_WEIGHT,
88+
weight: typeWeights.scalar,
9289
};
9390
} else if (
9491
isInterfaceType(fieldType) ||
@@ -105,7 +102,7 @@ function parseObjectFields(
105102
if (isScalarType(listType) && typeWeights.scalar === 0) {
106103
// list won't compound if weight is zero
107104
result.fields[field] = {
108-
weight: typeWeights.scalar || DEFAULT_SCALAR_WEIGHT,
105+
weight: typeWeights.scalar,
109106
};
110107
} else if (isEnumType(listType) && typeWeights.scalar === 0) {
111108
// list won't compound if weight of enum is zero
@@ -131,7 +128,7 @@ function parseObjectFields(
131128
);
132129
const weight = isCompositeType(listType)
133130
? typeWeightObject[listType.name.toLowerCase()].weight
134-
: typeWeights.scalar || DEFAULT_SCALAR_WEIGHT; // Note this includes enums
131+
: typeWeights.scalar; // Note this includes enums
135132
if (limitArg) {
136133
const node: ValueNode = limitArg.value;
137134
let multiplier = 1;
@@ -175,7 +172,7 @@ function parseObjectFields(
175172
* @param typeWeights
176173
* @returns
177174
*/
178-
function parseTypes(schema: GraphQLSchema, typeWeights: TypeWeightConfig): TypeWeightObject {
175+
function parseTypes(schema: GraphQLSchema, typeWeights: TypeWeightSet): TypeWeightObject {
179176
const typeMap: ObjMap<GraphQLNamedType> = schema.getTypeMap();
180177

181178
const result: TypeWeightObject = {};
@@ -193,13 +190,13 @@ function parseTypes(schema: GraphQLSchema, typeWeights: TypeWeightConfig): TypeW
193190
} else if (isEnumType(currentType)) {
194191
result[typeName] = {
195192
fields: {},
196-
weight: typeWeights.scalar || DEFAULT_SCALAR_WEIGHT,
193+
weight: typeWeights.scalar,
197194
};
198195
} else if (isUnionType(currentType)) {
199196
// FIXME: will need information on fields inorder calculate comlpextiy
200197
result[typeName] = {
201198
fields: {},
202-
weight: typeWeights.object || DEFAULT_OBJECT_WEIGHT,
199+
weight: typeWeights.object,
203200
};
204201
} else {
205202
// ? what else can get through here
@@ -230,7 +227,7 @@ function buildTypeWeightsFromSchema(
230227
if (!schema) throw new Error('Missing Argument: schema is required');
231228

232229
// Merge the provided type weights with the default to account for missing values
233-
const typeWeights: TypeWeightConfig = {
230+
const typeWeights: TypeWeightSet = {
234231
...defaultTypeWeightsConfig,
235232
...typeWeightsConfig,
236233
};

test/analysis/buildTypeWeights.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,18 @@ describe('Test buildTypeWeightsFromSchema function', () => {
565565
expect(typeWeightObject).toEqual(expectedOutput);
566566
});
567567

568+
test('object parameter set to 0', () => {
569+
const typeWeightObject = buildTypeWeightsFromSchema(schema, {
570+
object: 0,
571+
});
572+
573+
expectedOutput.user.weight = 0;
574+
expectedOutput.movie.weight = 0;
575+
// expectedOutput.query.weight = 2;
576+
577+
expect(typeWeightObject).toEqual(expectedOutput);
578+
});
579+
568580
test('scalar parameter', () => {
569581
const typeWeightObject = buildTypeWeightsFromSchema(schema, {
570582
scalar: 2,

test/analysis/typeComplexityAnalysis.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,11 @@ describe('Test getQueryTypeComplexity function', () => {
228228
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(2); // Query 1 + Scalars 1
229229
});
230230

231+
xtest('with one with capital first letter for field', () => {
232+
query = `query { Scalars { num } }`;
233+
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(2); // Query 1 + Scalars 1
234+
});
235+
231236
test('with two or more fields', () => {
232237
query = `query { scalars { num } test { name } }`;
233238
expect(getQueryTypeComplexity(parse(query), variables, typeWeights)).toBe(3); // Query 1 + scalars 1 + test 1

test/analysis/weightFunction.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ describe('Weight Function correctly parses Argument Nodes if', () => {
9797
});
9898
const query = `query { heroes(episode: NEWHOPE, first: 3) { stars, episode } }`;
9999
const queryAST: DocumentNode = parse(query);
100-
expect(getQueryTypeComplexity(queryAST, {}, customTypeWeights)).toBe(4); // 1 query and 4 enums
100+
expect(getQueryTypeComplexity(queryAST, {}, customTypeWeights)).toBe(1); // 1 query
101101
});
102102
test('a custom scalar weight was set to greater than 0', () => {
103103
const customTypeWeights: TypeWeightObject = buildTypeWeightsFromSchema(schema, {

0 commit comments

Comments
 (0)