From 8cfa3de8a12822efdaa52e43ecd07dea57b4f926 Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Tue, 27 Jun 2023 07:51:38 -0400 Subject: [PATCH] Implement OneOf Input Objects via `@oneOf` directive (#3513) Co-authored-by: Benjie Gillam Closes graphql/graphql-wg#648 --- src/__testUtils__/kitchenSinkSDL.ts | 6 + src/execution/__tests__/oneof-test.ts | 185 ++++++++++++++++++ src/index.ts | 1 + src/language/__tests__/schema-printer-test.ts | 6 + src/type/__tests__/introspection-test.ts | 106 ++++++++++ src/type/__tests__/validation-test.ts | 43 +++- src/type/definition.ts | 4 + src/type/directives.ts | 11 ++ src/type/index.ts | 1 + src/type/introspection.ts | 8 + src/type/validate.ts | 24 +++ .../__tests__/buildASTSchema-test.ts | 13 +- .../__tests__/coerceInputValue-test.ts | 105 ++++++++++ .../__tests__/findBreakingChanges-test.ts | 2 + src/utilities/__tests__/printSchema-test.ts | 4 + src/utilities/__tests__/valueFromAST-test.ts | 24 +++ src/utilities/coerceInputValue.ts | 24 +++ src/utilities/extendSchema.ts | 9 + src/utilities/valueFromAST.ts | 12 ++ .../__tests__/ValuesOfCorrectTypeRule-test.ts | 86 ++++++++ src/validation/__tests__/harness.ts | 6 + .../rules/ValuesOfCorrectTypeRule.ts | 78 +++++++- 22 files changed, 752 insertions(+), 6 deletions(-) create mode 100644 src/execution/__tests__/oneof-test.ts diff --git a/src/__testUtils__/kitchenSinkSDL.ts b/src/__testUtils__/kitchenSinkSDL.ts index cdf2f9afce..7b7a537783 100644 --- a/src/__testUtils__/kitchenSinkSDL.ts +++ b/src/__testUtils__/kitchenSinkSDL.ts @@ -27,6 +27,7 @@ type Foo implements Bar & Baz & Two { five(argument: [String] = ["string", "string"]): String six(argument: InputType = {key: "value"}): Type seven(argument: Int = null): Type + eight(argument: OneOfInputType): Type } type AnnotatedObject @onObject(arg: "value") { @@ -116,6 +117,11 @@ input InputType { answer: Int = 42 } +input OneOfInputType @oneOf { + string: String + int: Int +} + input AnnotatedInput @onInputObject { annotatedField: Type @onInputFieldDefinition } diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts new file mode 100644 index 0000000000..acde6031b4 --- /dev/null +++ b/src/execution/__tests__/oneof-test.ts @@ -0,0 +1,185 @@ +import { describe, it } from 'mocha'; + +import { expectJSON } from '../../__testUtils__/expectJSON.js'; + +import { parse } from '../../language/parser.js'; + +import { buildSchema } from '../../utilities/buildASTSchema.js'; + +import type { ExecutionResult } from '../execute.js'; +import { execute } from '../execute.js'; + +const schema = buildSchema(` + type Query { + test(input: TestInputObject!): TestObject + } + + input TestInputObject @oneOf { + a: String + b: Int + } + + type TestObject { + a: String + b: Int + } +`); + +function executeQuery( + query: string, + rootValue: unknown, + variableValues?: { [variable: string]: unknown }, +): ExecutionResult | Promise { + return execute({ schema, document: parse(query), rootValue, variableValues }); +} + +describe('Execute: Handles OneOf Input 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: + // This type of error would be caught at validation-time + // hence the vague error message here. + '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('accepts a good variable with an undefined key', () => { + const query = ` + query ($input: TestInputObject!) { + test(input: $input) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { + input: { a: 'abc', b: undefined }, + }); + + expectJSON(result).toDeepEqual({ + data: { + test: { + a: 'abc', + b: null, + }, + }, + }); + }); + + it('rejects a variable with multiple non-null keys', () => { + 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 for OneOf type "TestInputObject".', + }, + ], + }); + }); + + it('rejects a variable with multiple nullable keys', () => { + const query = ` + query ($input: TestInputObject!) { + test(input: $input) { + a + b + } + } + `; + const result = executeQuery(query, rootValue, { + input: { a: 'abc', b: null }, + }); + + expectJSON(result).toDeepEqual({ + errors: [ + { + locations: [{ column: 16, line: 2 }], + message: + 'Variable "$input" got invalid value { a: "abc", b: null }; Exactly one key must be specified for OneOf type "TestInputObject".', + }, + ], + }); + }); + }); +}); diff --git a/src/index.ts b/src/index.ts index 2817c6eb03..1a0f1b4c82 100644 --- a/src/index.ts +++ b/src/index.ts @@ -66,6 +66,7 @@ export { GraphQLStreamDirective, GraphQLDeprecatedDirective, GraphQLSpecifiedByDirective, + GraphQLOneOfDirective, // "Enum" of Type Kinds TypeKind, // Constant Deprecation Reason diff --git a/src/language/__tests__/schema-printer-test.ts b/src/language/__tests__/schema-printer-test.ts index 3de9865c2c..4f8166b7bc 100644 --- a/src/language/__tests__/schema-printer-test.ts +++ b/src/language/__tests__/schema-printer-test.ts @@ -61,6 +61,7 @@ describe('Printer: SDL document', () => { five(argument: [String] = ["string", "string"]): String six(argument: InputType = { key: "value" }): Type seven(argument: Int = null): Type + eight(argument: OneOfInputType): Type } type AnnotatedObject @onObject(arg: "value") { @@ -143,6 +144,11 @@ describe('Printer: SDL document', () => { answer: Int = 42 } + input OneOfInputType @oneOf { + string: String + int: Int + } + input AnnotatedInput @onInputObject { annotatedField: Type @onInputFieldDefinition } diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 1fa0518dd0..e6e5d0943e 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -372,6 +372,17 @@ describe('Introspection', () => { isDeprecated: false, deprecationReason: null, }, + { + name: 'isOneOf', + args: [], + type: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, + }, + isDeprecated: false, + deprecationReason: null, + }, ], inputFields: null, interfaces: [], @@ -989,6 +1000,12 @@ describe('Introspection', () => { }, ], }, + { + name: 'oneOf', + isRepeatable: false, + locations: ['INPUT_OBJECT'], + args: [], + }, ], }, }, @@ -1519,6 +1536,95 @@ describe('Introspection', () => { }); }); + it('identifies oneOf for input objects', () => { + const schema = buildSchema(` + input SomeInputObject @oneOf { + a: String + } + + input AnotherInputObject { + a: String + b: String + } + + type Query { + someField(someArg: SomeInputObject): String + anotherField(anotherArg: AnotherInputObject): String + } + `); + + const source = ` + { + oneOfInputObject: __type(name: "SomeInputObject") { + isOneOf + } + inputObject: __type(name: "AnotherInputObject") { + isOneOf + } + } + `; + + expect(graphqlSync({ schema, source })).to.deep.equal({ + data: { + oneOfInputObject: { + isOneOf: true, + }, + inputObject: { + isOneOf: false, + }, + }, + }); + }); + + it('returns null for oneOf for other types', () => { + const schema = buildSchema(` + type SomeObject implements SomeInterface { + fieldA: String + } + enum SomeEnum { + SomeObject + } + interface SomeInterface { + fieldA: String + } + union SomeUnion = SomeObject + type Query { + someField(enum: SomeEnum): SomeUnion + anotherField(enum: SomeEnum): SomeInterface + } + `); + + const source = ` + { + object: __type(name: "SomeObject") { + isOneOf + } + enum: __type(name: "SomeEnum") { + isOneOf + } + interface: __type(name: "SomeInterface") { + isOneOf + } + scalar: __type(name: "String") { + isOneOf + } + union: __type(name: "SomeUnion") { + isOneOf + } + } + `; + + expect(graphqlSync({ schema, source })).to.deep.equal({ + data: { + object: { isOneOf: null }, + enum: { isOneOf: null }, + interface: { isOneOf: null }, + scalar: { isOneOf: null }, + union: { isOneOf: null }, + }, + }); + }); + it('fails as expected on the __type root field without an arg', () => { const schema = buildSchema(` type Query { diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index a147a7e80f..7d215361f9 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -336,7 +336,7 @@ describe('Type System: A Schema must have Object root types', () => { ]); }); - it('rejects a schema extended with invalid root types', () => { + it('rejects a Schema extended with invalid root types', () => { let schema = buildSchema(` input SomeInputObject { test: String @@ -1749,6 +1749,47 @@ describe('Type System: Input Object fields must have input types', () => { }); }); +describe('Type System: OneOf Input Object fields must be nullable', () => { + it('rejects non-nullable fields', () => { + const schema = buildSchema(` + type Query { + test(arg: SomeInputObject): String + } + + input SomeInputObject @oneOf { + a: String + b: String! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: 'OneOf input field SomeInputObject.b must be nullable.', + locations: [{ line: 8, column: 12 }], + }, + ]); + }); + + it('rejects fields with default values', () => { + const schema = buildSchema(` + type Query { + test(arg: SomeInputObject): String + } + + input SomeInputObject @oneOf { + a: String + b: String = "foo" + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'OneOf input field SomeInputObject.b cannot have a default value.', + locations: [{ line: 8, column: 9 }], + }, + ]); + }); +}); + describe('Objects must adhere to Interface they implement', () => { it('accepts an Object which implements an Interface', () => { const schema = buildSchema(` diff --git a/src/type/definition.ts b/src/type/definition.ts index 25f4133a42..0ca4152bd2 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -1490,6 +1490,7 @@ export class GraphQLInputObjectType { extensions: Readonly; astNode: Maybe; extensionASTNodes: ReadonlyArray; + isOneOf: boolean; private _fields: ThunkObjMap; @@ -1499,6 +1500,7 @@ export class GraphQLInputObjectType { this.extensions = toObjMap(config.extensions); this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes ?? []; + this.isOneOf = config.isOneOf ?? false; this._fields = defineInputFieldMap.bind(undefined, config.fields); } @@ -1531,6 +1533,7 @@ export class GraphQLInputObjectType { extensions: this.extensions, astNode: this.astNode, extensionASTNodes: this.extensionASTNodes, + isOneOf: this.isOneOf, }; } @@ -1565,6 +1568,7 @@ export interface GraphQLInputObjectTypeConfig { extensions?: Maybe>; astNode?: Maybe; extensionASTNodes?: Maybe>; + isOneOf?: boolean; } interface GraphQLInputObjectTypeNormalizedConfig diff --git a/src/type/directives.ts b/src/type/directives.ts index 8fd5a6a62e..63acf6056e 100644 --- a/src/type/directives.ts +++ b/src/type/directives.ts @@ -247,6 +247,16 @@ export const GraphQLSpecifiedByDirective: GraphQLDirective = }, }); +/** + * Used to declare an Input Object as a OneOf Input Objects. + */ +export const GraphQLOneOfDirective: GraphQLDirective = new GraphQLDirective({ + name: 'oneOf', + description: 'Indicates an Input Object is a OneOf Input Object.', + locations: [DirectiveLocation.INPUT_OBJECT], + args: {}, +}); + /** * The full list of specified directives. */ @@ -256,6 +266,7 @@ export const specifiedDirectives: ReadonlyArray = GraphQLSkipDirective, GraphQLDeprecatedDirective, GraphQLSpecifiedByDirective, + GraphQLOneOfDirective, ]); export function isSpecifiedDirective(directive: GraphQLDirective): boolean { diff --git a/src/type/index.ts b/src/type/index.ts index 2cf94bdf04..6617a7bfe1 100644 --- a/src/type/index.ts +++ b/src/type/index.ts @@ -137,6 +137,7 @@ export { GraphQLStreamDirective, GraphQLDeprecatedDirective, GraphQLSpecifiedByDirective, + GraphQLOneOfDirective, // Constant Deprecation Reason DEFAULT_DEPRECATION_REASON, } from './directives.js'; diff --git a/src/type/introspection.ts b/src/type/introspection.ts index f5f593629d..5f3c9c1fa5 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -323,6 +323,14 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({ type: __Type, resolve: (type) => ('ofType' in type ? type.ofType : undefined), }, + isOneOf: { + type: GraphQLBoolean, + resolve: (type) => { + if (isInputObjectType(type)) { + return type.isOneOf; + } + }, + }, } as GraphQLFieldConfigMap), }); diff --git a/src/type/validate.ts b/src/type/validate.ts index 86461d5dde..deee23a372 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -542,6 +542,30 @@ function validateInputFields( [getDeprecatedDirectiveNode(field.astNode), field.astNode?.type], ); } + + if (inputObj.isOneOf) { + validateOneOfInputObjectField(inputObj, field, context); + } + } +} + +function validateOneOfInputObjectField( + type: GraphQLInputObjectType, + field: GraphQLInputField, + context: SchemaValidationContext, +): void { + if (isNonNullType(field.type)) { + context.reportError( + `OneOf input field ${type.name}.${field.name} must be nullable.`, + field.astNode?.type, + ); + } + + if (field.defaultValue !== undefined) { + context.reportError( + `OneOf input field ${type.name}.${field.name} cannot have a default value.`, + field.astNode, + ); } } diff --git a/src/utilities/__tests__/buildASTSchema-test.ts b/src/utilities/__tests__/buildASTSchema-test.ts index 6e152af36c..e199abc2b3 100644 --- a/src/utilities/__tests__/buildASTSchema-test.ts +++ b/src/utilities/__tests__/buildASTSchema-test.ts @@ -23,6 +23,7 @@ import { assertDirective, GraphQLDeprecatedDirective, GraphQLIncludeDirective, + GraphQLOneOfDirective, GraphQLSkipDirective, GraphQLSpecifiedByDirective, } from '../../type/directives.js'; @@ -222,7 +223,7 @@ describe('Schema Builder', () => { it('Maintains @include, @skip & @specifiedBy', () => { const schema = buildSchema('type Query'); - expect(schema.getDirectives()).to.have.lengthOf(4); + expect(schema.getDirectives()).to.have.lengthOf(5); expect(schema.getDirective('skip')).to.equal(GraphQLSkipDirective); expect(schema.getDirective('include')).to.equal(GraphQLIncludeDirective); expect(schema.getDirective('deprecated')).to.equal( @@ -231,6 +232,7 @@ describe('Schema Builder', () => { expect(schema.getDirective('specifiedBy')).to.equal( GraphQLSpecifiedByDirective, ); + expect(schema.getDirective('oneOf')).to.equal(GraphQLOneOfDirective); }); it('Overriding directives excludes specified', () => { @@ -239,9 +241,10 @@ describe('Schema Builder', () => { directive @include on FIELD directive @deprecated on FIELD_DEFINITION directive @specifiedBy on FIELD_DEFINITION + directive @oneOf on OBJECT `); - expect(schema.getDirectives()).to.have.lengthOf(4); + expect(schema.getDirectives()).to.have.lengthOf(5); expect(schema.getDirective('skip')).to.not.equal(GraphQLSkipDirective); expect(schema.getDirective('include')).to.not.equal( GraphQLIncludeDirective, @@ -252,18 +255,20 @@ describe('Schema Builder', () => { expect(schema.getDirective('specifiedBy')).to.not.equal( GraphQLSpecifiedByDirective, ); + expect(schema.getDirective('oneOf')).to.not.equal(GraphQLOneOfDirective); }); - it('Adding directives maintains @include, @skip & @specifiedBy', () => { + it('Adding directives maintains @include, @skip, @deprecated, @specifiedBy, and @oneOf', () => { const schema = buildSchema(` directive @foo(arg: Int) on FIELD `); - expect(schema.getDirectives()).to.have.lengthOf(5); + expect(schema.getDirectives()).to.have.lengthOf(6); expect(schema.getDirective('skip')).to.not.equal(undefined); expect(schema.getDirective('include')).to.not.equal(undefined); expect(schema.getDirective('deprecated')).to.not.equal(undefined); expect(schema.getDirective('specifiedBy')).to.not.equal(undefined); + expect(schema.getDirective('oneOf')).to.not.equal(undefined); }); it('Type modifiers', () => { diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index 5e0199fead..7712c91a39 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -273,6 +273,111 @@ 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 for OneOf type "TestInputObject".', + 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 for OneOf type "TestInputObject".', + 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 for OneOf type "TestInputObject".', + path: [], + value: { bart: 123 }, + }, + ]); + }); + }); + describe('for GraphQLInputObject with default value', () => { const makeTestInputObject = (defaultValue: any) => new GraphQLInputObjectType({ diff --git a/src/utilities/__tests__/findBreakingChanges-test.ts b/src/utilities/__tests__/findBreakingChanges-test.ts index dbd19cb893..5d92522fe3 100644 --- a/src/utilities/__tests__/findBreakingChanges-test.ts +++ b/src/utilities/__tests__/findBreakingChanges-test.ts @@ -4,6 +4,7 @@ import { describe, it } from 'mocha'; import { GraphQLDeprecatedDirective, GraphQLIncludeDirective, + GraphQLOneOfDirective, GraphQLSkipDirective, GraphQLSpecifiedByDirective, } from '../../type/directives.js'; @@ -802,6 +803,7 @@ describe('findBreakingChanges', () => { GraphQLSkipDirective, GraphQLIncludeDirective, GraphQLSpecifiedByDirective, + GraphQLOneOfDirective, ], }); diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 2da58e7d06..8940cafbe5 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -795,6 +795,9 @@ describe('Type System Printer', () => { url: String! ) on SCALAR + """Indicates an Input Object is a OneOf Input Object.""" + directive @oneOf on INPUT_OBJECT + """ A GraphQL Schema defines the capabilities of a GraphQL server. It exposes all available types and directives on the server, as well as the entry points for query, mutation, and subscription operations. """ @@ -837,6 +840,7 @@ describe('Type System Printer', () => { enumValues(includeDeprecated: Boolean = false): [__EnumValue!] inputFields(includeDeprecated: Boolean = false): [__InputValue!] ofType: __Type + isOneOf: Boolean } """An enum describing what kind of type a given \`__Type\` is.""" diff --git a/src/utilities/__tests__/valueFromAST-test.ts b/src/utilities/__tests__/valueFromAST-test.ts index d0a8380d13..e287d91691 100644 --- a/src/utilities/__tests__/valueFromAST-test.ts +++ b/src/utilities/__tests__/valueFromAST-test.ts @@ -195,6 +195,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); @@ -220,6 +228,22 @@ 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('{ b: "def" }', testOneOfInputObj).to.deep.equal({ + b: 'def', + }); + expectValueFrom('{ a: "abc", b: null }', testOneOfInputObj).to.deep.equal( + undefined, + ); + 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', () => { diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index eebddcba83..9aa49abed5 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -142,6 +142,30 @@ 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 for OneOf type "${type.name}".`, + ), + ); + } + + 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; } diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index 831733b69b..5a8cba8865 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -62,6 +62,7 @@ import { import { GraphQLDeprecatedDirective, GraphQLDirective, + GraphQLOneOfDirective, GraphQLSpecifiedByDirective, isSpecifiedDirective, } from '../type/directives.js'; @@ -696,6 +697,7 @@ export function extendSchemaImpl( fields: () => buildInputFieldMap(allNodes), astNode, extensionASTNodes, + isOneOf: isOneOf(astNode), }); } } @@ -732,3 +734,10 @@ function getSpecifiedByURL( // @ts-expect-error validated by `getDirectiveValues` return specifiedBy?.url; } + +/** + * Given an input object node, returns if the node should be OneOf. + */ +function isOneOf(node: InputObjectTypeDefinitionNode): boolean { + return Boolean(getDirectiveValues(GraphQLOneOfDirective, node)); +} diff --git a/src/utilities/valueFromAST.ts b/src/utilities/valueFromAST.ts index eeb538d2a8..5e0d3c517e 100644 --- a/src/utilities/valueFromAST.ts +++ b/src/utilities/valueFromAST.ts @@ -126,6 +126,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; } diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts index f0b7dfa57e..c36ebb6992 100644 --- a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts +++ b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts @@ -874,6 +874,28 @@ describe('Validate: Values of correct type', () => { }); }); + describe('Valid oneOf input object value', () => { + it('Exactly one field', () => { + expectValid(` + { + complicatedArgs { + oneOfArgField(oneOfArg: { stringField: "abc" }) + } + } + `); + }); + + it('Exactly one non-nullable variable', () => { + expectValid(` + query ($string: String!) { + complicatedArgs { + oneOfArgField(oneOfArg: { stringField: $string }) + } + } + `); + }); + }); + describe('Invalid input object value', () => { it('Partial object, missing required', () => { expectErrors(` @@ -1041,6 +1063,70 @@ describe('Validate: Values of correct type', () => { }); }); + describe('Invalid oneOf input object value', () => { + it('Invalid field type', () => { + expectErrors(` + { + complicatedArgs { + oneOfArgField(oneOfArg: { stringField: 2 }) + } + } + `).toDeepEqual([ + { + message: 'String cannot represent a non string value: 2', + locations: [{ line: 4, column: 52 }], + }, + ]); + }); + + it('Exactly one null field', () => { + expectErrors(` + { + complicatedArgs { + oneOfArgField(oneOfArg: { stringField: null }) + } + } + `).toDeepEqual([ + { + message: 'Field "OneOfInput.stringField" must be non-null.', + locations: [{ line: 4, column: 37 }], + }, + ]); + }); + + it('Exactly one nullable variable', () => { + expectErrors(` + query ($string: String) { + complicatedArgs { + oneOfArgField(oneOfArg: { stringField: $string }) + } + } + `).toDeepEqual([ + { + message: + 'Variable "string" must be non-nullable to be used for OneOf Input Object "OneOfInput".', + locations: [{ line: 4, column: 37 }], + }, + ]); + }); + + it('More than one field', () => { + expectErrors(` + { + complicatedArgs { + oneOfArgField(oneOfArg: { stringField: "abc", intField: 123 }) + } + } + `).toDeepEqual([ + { + message: + 'OneOf Input Object "OneOfInput" must specify exactly one key.', + locations: [{ line: 4, column: 37 }], + }, + ]); + }); + }); + describe('Directive arguments', () => { it('with directives of valid types', () => { expectValid(` diff --git a/src/validation/__tests__/harness.ts b/src/validation/__tests__/harness.ts index 682932d897..b7710ff9d9 100644 --- a/src/validation/__tests__/harness.ts +++ b/src/validation/__tests__/harness.ts @@ -82,6 +82,11 @@ export const testSchema: GraphQLSchema = buildSchema(` stringListField: [String] } + input OneOfInput @oneOf { + stringField: String + intField: Int + } + type ComplicatedArgs { # TODO List # TODO Coercion @@ -96,6 +101,7 @@ export const testSchema: GraphQLSchema = buildSchema(` stringListArgField(stringListArg: [String]): String stringListNonNullArgField(stringListNonNullArg: [String!]): String complexArgField(complexArg: ComplexInput): String + oneOfArgField(oneOfArg: OneOfInput): String multipleReqs(req1: Int!, req2: Int!): String nonNullFieldWithDefault(arg: Int! = 0): String multipleOpts(opt1: Int = 0, opt2: Int = 0): String diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 09a7316787..3b9a1ee645 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -4,10 +4,17 @@ import { suggestionList } from '../../jsutils/suggestionList.js'; import { GraphQLError } from '../../error/GraphQLError.js'; -import type { ValueNode } from '../../language/ast.js'; +import type { + ObjectFieldNode, + ObjectValueNode, + ValueNode, + VariableDefinitionNode, +} from '../../language/ast.js'; +import { Kind } from '../../language/kinds.js'; import { print } from '../../language/printer.js'; import type { ASTVisitor } from '../../language/visitor.js'; +import type { GraphQLInputObjectType } from '../../type/definition.js'; import { getNamedType, getNullableType, @@ -31,7 +38,17 @@ import type { ValidationContext } from '../ValidationContext.js'; export function ValuesOfCorrectTypeRule( context: ValidationContext, ): ASTVisitor { + let variableDefinitions: { [key: string]: VariableDefinitionNode } = {}; + return { + OperationDefinition: { + enter() { + variableDefinitions = {}; + }, + }, + VariableDefinition(definition) { + variableDefinitions[definition.variable.name.value] = definition; + }, ListValue(node) { // Note: TypeInfo will traverse into a list's item type, so look to the // parent input type to check if it is a list. @@ -63,6 +80,16 @@ export function ValuesOfCorrectTypeRule( ); } } + + if (type.isOneOf) { + validateOneOfInputObject( + context, + node, + type, + fieldNodeMap, + variableDefinitions, + ); + } }, ObjectField(node) { const parentType = getNamedType(context.getParentInputType()); @@ -152,3 +179,52 @@ function isValidValueNode(context: ValidationContext, node: ValueNode): void { } } } + +function validateOneOfInputObject( + context: ValidationContext, + node: ObjectValueNode, + type: GraphQLInputObjectType, + fieldNodeMap: Map, + variableDefinitions: { [key: string]: VariableDefinitionNode }, +): void { + const keys = Array.from(fieldNodeMap.keys()); + const isNotExactlyOneField = keys.length !== 1; + + if (isNotExactlyOneField) { + context.reportError( + new GraphQLError( + `OneOf Input Object "${type.name}" must specify exactly one key.`, + { nodes: [node] }, + ), + ); + return; + } + + const value = fieldNodeMap.get(keys[0])?.value; + const isNullLiteral = !value || value.kind === Kind.NULL; + const isVariable = value?.kind === Kind.VARIABLE; + + if (isNullLiteral) { + context.reportError( + new GraphQLError(`Field "${type.name}.${keys[0]}" must be non-null.`, { + nodes: [node], + }), + ); + return; + } + + if (isVariable) { + const variableName = value.name.value; + const definition = variableDefinitions[variableName]; + const isNullableVariable = definition.type.kind !== Kind.NON_NULL_TYPE; + + if (isNullableVariable) { + context.reportError( + new GraphQLError( + `Variable "${variableName}" must be non-nullable to be used for OneOf Input Object "${type.name}".`, + { nodes: [node] }, + ), + ); + } + } +}