Skip to content

Commit

Permalink
Updated to most recent version of spec proposal.
Browse files Browse the repository at this point in the history 
This is now *a breaking change*. The default validation rules are stricter, however with a configuration flag the previous lax behavior can be used which will ensure an existing service can support all existing incoming operations.

For example to continue to support existing queries after updating to the new version, replace:

```js
graphql({ schema, source })
```

With:

```js
graphql({ schema, source, options: {
  allowNullableVariablesInNonNullPositions: true
}})
```

Another more minor breaking change is that the final `typeInfo` argument to `validate` has moved positions. However very few should be reliant on this experimental arg.
  • Loading branch information
@leebyron
leebyron committed Apr 6, 2018
1 parent 1afc4dc commit 246b998
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 70 deletions.
22 changes: 18 additions & 4 deletions src/graphql.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@

import { validateSchema } from './type/validate';
import { parse } from './language/parser';
import { validate } from './validation/validate';
import { validate, specifiedRules } from './validation';
import { execute } from './execution/execute';
import type { ObjMap } from './jsutils/ObjMap';
import type { Source } from './language/source';
import type { ParseOptions, Source } from './language';
import type { GraphQLFieldResolver } from './type/definition';
import type { GraphQLSchema } from './type/schema';
import type { ExecutionResult } from './execution/execute';
import type { ValidationOptions } from './validation';
import type { MaybePromise } from './jsutils/MaybePromise';

/**
Expand Down Expand Up @@ -47,6 +48,9 @@ import type { MaybePromise } from './jsutils/MaybePromise';
* A resolver function to use when one is not provided by the schema.
* If not provided, the default field resolver is used (which looks for a
* value or method on the source value with the field's name).
* options:
* An additional set of flags which enable legacy, compataibility, or
* experimental behavior.
*/
export type GraphQLArgs = {|
schema: GraphQLSchema,
Expand All @@ -56,6 +60,7 @@ export type GraphQLArgs = {|
variableValues?: ?ObjMap<mixed>,
operationName?: ?string,
fieldResolver?: ?GraphQLFieldResolver<any, any>,
options?: ParseOptions & ValidationOptions,
|};
declare function graphql(GraphQLArgs, ..._: []): Promise<ExecutionResult>;
/* eslint-disable no-redeclare */
Expand All @@ -67,6 +72,7 @@ declare function graphql(
variableValues?: ?ObjMap<mixed>,
operationName?: ?string,
fieldResolver?: ?GraphQLFieldResolver<any, any>,
options?: ParseOptions & ValidationOptions,
): Promise<ExecutionResult>;
export function graphql(
argsOrSchema,
Expand All @@ -76,6 +82,7 @@ export function graphql(
variableValues,
operationName,
fieldResolver,
options,
) {
/* eslint-enable no-redeclare */
// Always return a Promise for a consistent API.
Expand All @@ -91,6 +98,7 @@ export function graphql(
argsOrSchema.variableValues,
argsOrSchema.operationName,
argsOrSchema.fieldResolver,
argsOrSchema.options,
)
: graphqlImpl(
argsOrSchema,
Expand All @@ -100,6 +108,7 @@ export function graphql(
variableValues,
operationName,
fieldResolver,
options,
),
),
);
Expand All @@ -121,6 +130,7 @@ declare function graphqlSync(
variableValues?: ?ObjMap<mixed>,
operationName?: ?string,
fieldResolver?: ?GraphQLFieldResolver<any, any>,
options?: ParseOptions & ValidationOptions,
): ExecutionResult;
export function graphqlSync(
argsOrSchema,
Expand All @@ -130,6 +140,7 @@ export function graphqlSync(
variableValues,
operationName,
fieldResolver,
options,
) {
// Extract arguments from object args if provided.
const result =
Expand All @@ -142,6 +153,7 @@ export function graphqlSync(
argsOrSchema.variableValues,
argsOrSchema.operationName,
argsOrSchema.fieldResolver,
argsOrSchema.options,
)
: graphqlImpl(
argsOrSchema,
Expand All @@ -151,6 +163,7 @@ export function graphqlSync(
variableValues,
operationName,
fieldResolver,
options,
);

// Assert that the execution was synchronous.
Expand All @@ -169,6 +182,7 @@ function graphqlImpl(
variableValues,
operationName,
fieldResolver,
options,
): MaybePromise<ExecutionResult> {
// Validate Schema
const schemaValidationErrors = validateSchema(schema);
Expand All @@ -179,13 +193,13 @@ function graphqlImpl(
// Parse
let document;
try {
document = parse(source);
document = parse(source, options);
} catch (syntaxError) {
return { errors: [syntaxError] };
}

// Validate
const validationErrors = validate(schema, document);
const validationErrors = validate(schema, document, specifiedRules, options);
if (validationErrors.length > 0) {
return { errors: validationErrors };
}
Expand Down
1 change: 1 addition & 0 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ export { subscribe, createSourceEventStream } from './subscription';
// Validate GraphQL queries.
export {
validate,
ValidationOptions,
ValidationContext,
// All validation rules in the GraphQL Specification.
specifiedRules,
Expand Down
18 changes: 18 additions & 0 deletions src/validation/ValidationContext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ import { TypeInfo } from '../utilities/TypeInfo';
type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode;
type VariableUsage = { node: VariableNode, type: ?GraphQLInputType };

/**
* Configuration options to control validation behavior.
*/
export type ValidationOptions = {
/**
* If enabled, validation will allow nullable variables with default values
* to be used in non-null positions. Earlier versions explicitly allowed such
* operations due to a slightly different interpretation of default values and
* null values. GraphQL services accepting operations written before this
* version may continue to allow such operations by enabling this option,
* however GraphQL services established after this version should not.
*/
allowNullableVariablesInNonNullPositions?: boolean,
};

/**
* An instance of this class is passed as the "this" context to all validators,
* allowing access to commonly useful contextual information from within a
Expand All @@ -42,6 +57,7 @@ export default class ValidationContext {
_schema: GraphQLSchema;
_ast: DocumentNode;
_typeInfo: TypeInfo;
options: ValidationOptions;
_errors: Array<GraphQLError>;
_fragments: ObjMap<FragmentDefinitionNode>;
_fragmentSpreads: Map<SelectionSetNode, $ReadOnlyArray<FragmentSpreadNode>>;
Expand All @@ -59,10 +75,12 @@ export default class ValidationContext {
schema: GraphQLSchema,
ast: DocumentNode,
typeInfo: TypeInfo,
options?: ValidationOptions,
): void {
this._schema = schema;
this._ast = ast;
this._typeInfo = typeInfo;
this.options = options || {};
this._errors = [];
this._fragmentSpreads = new Map();
this._recursivelyReferencedFragments = new Map();
Expand Down
122 changes: 75 additions & 47 deletions src/validation/__tests__/VariablesInAllowedPosition-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
*/

import { describe, it } from 'mocha';
import { expectPassesRule, expectFailsRule } from './harness';
import {
expectPassesRule,
expectFailsRule,
expectPassesRuleWithOptions,
expectFailsRuleWithOptions,
} from './harness';
import {
VariablesInAllowedPosition,
badVarPosMessage,
Expand Down Expand Up @@ -91,20 +96,6 @@ describe('Validate: Variables are in allowed positions', () => {
);
});

it('Int => Int! with non-null default value', () => {
expectPassesRule(
VariablesInAllowedPosition,
`
query Query($intArg: Int = 1)
{
complicatedArgs {
nonNullIntArgField(nonNullIntArg: $intArg)
}
}
`,
);
});

it('[String] => [String]', () => {
expectPassesRule(
VariablesInAllowedPosition,
Expand Down Expand Up @@ -201,18 +192,6 @@ describe('Validate: Variables are in allowed positions', () => {
);
});

it('Boolean => Boolean! in directive with default', () => {
expectPassesRule(
VariablesInAllowedPosition,
`
query Query($boolVar: Boolean = false)
{
dog @include(if: $boolVar)
}
`,
);
});

it('Int => Int!', () => {
expectFailsRule(
VariablesInAllowedPosition,
Expand All @@ -232,26 +211,6 @@ describe('Validate: Variables are in allowed positions', () => {
);
});

it('Int => Int! with null default value', () => {
expectFailsRule(
VariablesInAllowedPosition,
`
query Query($intArg: Int = null) {
complicatedArgs {
nonNullIntArgField(nonNullIntArg: $intArg)
}
}
`,
[
{
message: badVarPosMessage('intArg', 'Int', 'Int!'),
locations: [{ line: 2, column: 19 }, { line: 4, column: 45 }],
path: undefined,
},
],
);
});

it('Int => Int! within fragment', () => {
expectFailsRule(
VariablesInAllowedPosition,
Expand Down Expand Up @@ -393,4 +352,73 @@ describe('Validate: Variables are in allowed positions', () => {
],
);
});

describe('allowNullableVariablesInNonNullPositions', () => {
it('Int => Int! with non-null default value without option', () => {
expectFailsRule(
VariablesInAllowedPosition,
`
query Query($intVar: Int = 1) {
complicatedArgs {
nonNullIntArgField(nonNullIntArg: $intVar)
}
}
`,
[
{
message: badVarPosMessage('intVar', 'Int', 'Int!'),
locations: [{ line: 2, column: 21 }, { line: 4, column: 47 }],
path: undefined,
},
],
);
});

it('Int => Int! with null default value fails with option', () => {
expectFailsRuleWithOptions(
{ allowNullableVariablesInNonNullPositions: true },
VariablesInAllowedPosition,
`
query Query($intVar: Int = null) {
complicatedArgs {
nonNullIntArgField(nonNullIntArg: $intVar)
}
}
`,
[
{
message: badVarPosMessage('intVar', 'Int', 'Int!'),
locations: [{ line: 2, column: 23 }, { line: 4, column: 49 }],
path: undefined,
},
],
);
});

it('Int => Int! with non-null default value with option', () => {
expectPassesRuleWithOptions(
{ allowNullableVariablesInNonNullPositions: true },
VariablesInAllowedPosition,
`
query Query($intVar: Int = 1) {
complicatedArgs {
nonNullIntArgField(nonNullIntArg: $intVar)
}
}
`,
);
});

it('Boolean => Boolean! in directive with default value with option', () => {
expectPassesRuleWithOptions(
{ allowNullableVariablesInNonNullPositions: true },
VariablesInAllowedPosition,
`
query Query($boolVar: Boolean = false) {
dog @include(if: $boolVar)
}
`,
);
});
});
});
18 changes: 14 additions & 4 deletions src/validation/__tests__/harness.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,15 @@ export const testSchema = new GraphQLSchema({
],
});

function expectValid(schema, rules, queryString) {
const errors = validate(schema, parse(queryString), rules);
function expectValid(schema, rules, queryString, options) {
const ast = parse(queryString);
const errors = validate(schema, ast, rules, options);
expect(errors).to.deep.equal([], 'Should validate');
}

function expectInvalid(schema, rules, queryString, expectedErrors) {
const errors = validate(schema, parse(queryString), rules);
function expectInvalid(schema, rules, queryString, expectedErrors, options) {
const ast = parse(queryString);
const errors = validate(schema, ast, rules, options);
expect(errors).to.have.length.of.at.least(1, 'Should not validate');
expect(errors).to.deep.equal(expectedErrors);
return errors;
Expand All @@ -441,6 +443,14 @@ export function expectFailsRule(rule, queryString, errors) {
return expectInvalid(testSchema, [rule], queryString, errors);
}

export function expectPassesRuleWithOptions(options, rule, queryString) {
return expectValid(testSchema, [rule], queryString, options);
}

export function expectFailsRuleWithOptions(options, rule, queryString, errors) {
return expectInvalid(testSchema, [rule], queryString, errors, options);
}

export function expectPassesRuleWithSchema(schema, rule, queryString, errors) {
return expectValid(schema, [rule], queryString, errors);
}
Expand Down
2 changes: 1 addition & 1 deletion src/validation/__tests__/validation-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('Validate: Supports full validation', () => {
}
`);

const errors = validate(testSchema, ast, specifiedRules, typeInfo);
const errors = validate(testSchema, ast, specifiedRules, {}, typeInfo);

const errorMessages = errors.map(err => err.message);

Expand Down
1 change: 1 addition & 0 deletions src/validation/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export { validate } from './validate';
// https://github.com/tc39/proposal-export-default-from
import ValidationContext from './ValidationContext';
export { ValidationContext };
export type { ValidationOptions } from './ValidationContext';

export { specifiedRules } from './specifiedRules';

Expand Down
Loading

0 comments on commit 246b998

Please sign in to comment.