Skip to content

Commit

Permalink
Validate oneOf input objects at execution time
Browse files Browse the repository at this point in the history
This ensures that we enforce the rules for OneOf Input Objects at
execution time.
  • Loading branch information
erikkessler1 committed Mar 18, 2022
1 parent 24f0d95 commit 15e39f5
Show file tree
Hide file tree
Showing 5 changed files with 294 additions and 0 deletions.
140 changes: 140 additions & 0 deletions src/execution/__tests__/oneof-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { describe, it } from 'mocha';

import { expectJSON } from '../../__testUtils__/expectJSON';

import { parse } from '../../language/parser';

import { buildSchema } from '../../utilities/buildASTSchema';

import type { ExecutionResult } from '../execute';
import { execute } from '../execute';

const schema = buildSchema(`
type Query {
test(input: TestInputObject!): TestObject
}
input TestInputObject @oneOf {
a: String
b: Int
}
type TestObject @oneOf {
a: String
b: Int
}
schema {
query: Query
}
`);

function executeQuery(
query: string,
rootValue: unknown,
variableValues?: { [variable: string]: unknown },
): ExecutionResult | Promise<ExecutionResult> {
return execute({ schema, document: parse(query), rootValue, variableValues });
}

describe('Execute: Handles Oneof Input Objects and Oneof Objects', () => {
describe('Oneof Input Objects', () => {
const rootValue = {
test({ input }: { input: { a?: string; b?: number } }) {
return input;
},
};

it('accepts a good default value', () => {
const query = `
query ($input: TestInputObject! = {a: "abc"}) {
test(input: $input) {
a
b
}
}
`;
const result = executeQuery(query, rootValue);

expectJSON(result).toDeepEqual({
data: {
test: {
a: 'abc',
b: null,
},
},
});
});

it('rejects a bad default value', () => {
const query = `
query ($input: TestInputObject! = {a: "abc", b: 123}) {
test(input: $input) {
a
b
}
}
`;
const result = executeQuery(query, rootValue);

expectJSON(result).toDeepEqual({
data: {
test: null,
},
errors: [
{
locations: [{ column: 23, line: 3 }],
message:
'Argument "input" of non-null type "TestInputObject!" must not be null.',
path: ['test'],
},
],
});
});

it('accepts a good variable', () => {
const query = `
query ($input: TestInputObject!) {
test(input: $input) {
a
b
}
}
`;
const result = executeQuery(query, rootValue, { input: { a: 'abc' } });

expectJSON(result).toDeepEqual({
data: {
test: {
a: 'abc',
b: null,
},
},
});
});

it('rejects a bad variable', () => {
const query = `
query ($input: TestInputObject!) {
test(input: $input) {
a
b
}
}
`;
const result = executeQuery(query, rootValue, {
input: { a: 'abc', b: 123 },
});

expectJSON(result).toDeepEqual({
errors: [
{
locations: [{ column: 16, line: 2 }],
message:
'Variable "$input" got invalid value { a: "abc", b: 123 }; Exactly one key must be specified.',
},
],
});
});
});
});
102 changes: 102 additions & 0 deletions src/utilities/__tests__/coerceInputValue-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,108 @@ describe('coerceInputValue', () => {
});
});

describe('for GraphQLInputObject that isOneOf', () => {
const TestInputObject = new GraphQLInputObjectType({
name: 'TestInputObject',
fields: {
foo: { type: GraphQLInt },
bar: { type: GraphQLInt },
},
isOneOf: true,
});

it('returns no error for a valid input', () => {
const result = coerceValue({ foo: 123 }, TestInputObject);
expectValue(result).to.deep.equal({ foo: 123 });
});

it('returns an error if more than one field is specified', () => {
const result = coerceValue({ foo: 123, bar: null }, TestInputObject);
expectErrors(result).to.deep.equal([
{
error: 'Exactly one key must be specified.',
path: [],
value: { foo: 123, bar: null },
},
]);
});

it('returns an error the one field is null', () => {
const result = coerceValue({ bar: null }, TestInputObject);
expectErrors(result).to.deep.equal([
{
error: 'Field "bar" must be non-null.',
path: ['bar'],
value: null,
},
]);
});

it('returns an error for an invalid field', () => {
const result = coerceValue({ foo: NaN }, TestInputObject);
expectErrors(result).to.deep.equal([
{
error: 'Int cannot represent non-integer value: NaN',
path: ['foo'],
value: NaN,
},
]);
});

it('returns multiple errors for multiple invalid fields', () => {
const result = coerceValue({ foo: 'abc', bar: 'def' }, TestInputObject);
expectErrors(result).to.deep.equal([
{
error: 'Int cannot represent non-integer value: "abc"',
path: ['foo'],
value: 'abc',
},
{
error: 'Int cannot represent non-integer value: "def"',
path: ['bar'],
value: 'def',
},
{
error: 'Exactly one key must be specified.',
path: [],
value: { foo: 'abc', bar: 'def' },
},
]);
});

it('returns error for an unknown field', () => {
const result = coerceValue(
{ foo: 123, unknownField: 123 },
TestInputObject,
);
expectErrors(result).to.deep.equal([
{
error:
'Field "unknownField" is not defined by type "TestInputObject".',
path: [],
value: { foo: 123, unknownField: 123 },
},
]);
});

it('returns error for a misspelled field', () => {
const result = coerceValue({ bart: 123 }, TestInputObject);
expectErrors(result).to.deep.equal([
{
error:
'Field "bart" is not defined by type "TestInputObject". Did you mean "bar"?',
path: [],
value: { bart: 123 },
},
{
error: 'Exactly one key must be specified.',
path: [],
value: { bart: 123 },
},
]);
});
});

describe('for GraphQLInputObject with default value', () => {
const makeTestInputObject = (defaultValue: any) =>
new GraphQLInputObjectType({
Expand Down
18 changes: 18 additions & 0 deletions src/utilities/__tests__/valueFromAST-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,14 @@ describe('valueFromAST', () => {
requiredBool: { type: nonNullBool },
},
});
const testOneOfInputObj = new GraphQLInputObjectType({
name: 'TestOneOfInput',
fields: {
a: { type: GraphQLString },
b: { type: GraphQLString },
},
isOneOf: true,
});

it('coerces input objects according to input coercion rules', () => {
expectValueFrom('null', testInputObj).to.equal(null);
Expand All @@ -221,6 +229,16 @@ describe('valueFromAST', () => {
);
expectValueFrom('{ requiredBool: null }', testInputObj).to.equal(undefined);
expectValueFrom('{ bool: true }', testInputObj).to.equal(undefined);
expectValueFrom('{ a: "abc" }', testOneOfInputObj).to.deep.equal({
a: 'abc',
});
expectValueFrom('{ a: null }', testOneOfInputObj).to.equal(undefined);
expectValueFrom('{ a: 1 }', testOneOfInputObj).to.equal(undefined);
expectValueFrom('{ a: "abc", b: "def" }', testOneOfInputObj).to.equal(
undefined,
);
expectValueFrom('{}', testOneOfInputObj).to.equal(undefined);
expectValueFrom('{ c: "abc" }', testOneOfInputObj).to.equal(undefined);
});

it('accepts variable values assuming already coerced', () => {
Expand Down
22 changes: 22 additions & 0 deletions src/utilities/coerceInputValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,28 @@ function coerceInputValueImpl(
);
}
}

if (type.isOneOf) {
const keys = Object.keys(coercedValue);
if (keys.length !== 1) {
onError(
pathToArray(path),
inputValue,
new GraphQLError('Exactly one key must be specified.'),
);
}

const key = keys[0];
const value = coercedValue[key];
if (value === null) {
onError(
pathToArray(path).concat(key),
value,
new GraphQLError(`Field "${key}" must be non-null.`),
);
}
}

return coercedValue;
}

Expand Down
12 changes: 12 additions & 0 deletions src/utilities/valueFromAST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,18 @@ export function valueFromAST(
}
coercedObj[field.name] = fieldValue;
}

if (type.isOneOf) {
const keys = Object.keys(coercedObj);
if (keys.length !== 1) {
return; // Invalid: not exactly one key, intentionally return no value.
}

if (coercedObj[keys[0]] === null) {
return; // Invalid: value not non-null, intentionally return no value.
}
}

return coercedObj;
}

Expand Down

0 comments on commit 15e39f5

Please sign in to comment.