From 99189d9a6baf811ca9e4e4ce691918703e3f308f Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Fri, 14 Jan 2022 12:17:09 -0500 Subject: [PATCH 01/16] Add `oneOf` directive This adds the @oneOf directive as a default directive to indicate Objects and Input Objects as OneOf Objects/Input Objects which ensure there is exactly one non-null entry. Future commits will work to implement this directive. --- src/index.ts | 1 + src/type/__tests__/introspection-test.ts | 6 ++++++ src/type/directives.ts | 12 ++++++++++++ src/type/index.ts | 1 + src/utilities/__tests__/buildASTSchema-test.ts | 13 +++++++++---- src/utilities/__tests__/findBreakingChanges-test.ts | 2 ++ src/utilities/__tests__/printSchema-test.ts | 5 +++++ 7 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/index.ts b/src/index.ts index 7fbf4d6d68..8587cf470a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -64,6 +64,7 @@ export { GraphQLSkipDirective, GraphQLDeprecatedDirective, GraphQLSpecifiedByDirective, + GraphQLOneOfDirective, // "Enum" of Type Kinds TypeKind, // Constant Deprecation Reason diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 741d9c0076..de1344901d 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -989,6 +989,12 @@ describe('Introspection', () => { }, ], }, + { + name: 'oneOf', + isRepeatable: false, + locations: ['OBJECT', 'INPUT_OBJECT'], + args: [], + }, ], }, }, diff --git a/src/type/directives.ts b/src/type/directives.ts index bb3e441a43..b225d6062f 100644 --- a/src/type/directives.ts +++ b/src/type/directives.ts @@ -209,6 +209,17 @@ export const GraphQLSpecifiedByDirective: GraphQLDirective = }, }); +/** + * Used to declare an Input Object as a OneOf Input Objects and an Object as a OneOf Object. + */ +export const GraphQLOneOfDirective: GraphQLDirective = new GraphQLDirective({ + name: 'oneOf', + description: + 'Indicates an Object is a OneOf Object or an Input Object is a OneOf Input Object.', + locations: [DirectiveLocation.OBJECT, DirectiveLocation.INPUT_OBJECT], + args: {}, +}); + /** * The full list of specified directives. */ @@ -218,6 +229,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 270dd67d35..cf276d1e02 100644 --- a/src/type/index.ts +++ b/src/type/index.ts @@ -133,6 +133,7 @@ export { GraphQLSkipDirective, GraphQLDeprecatedDirective, GraphQLSpecifiedByDirective, + GraphQLOneOfDirective, // Constant Deprecation Reason DEFAULT_DEPRECATION_REASON, } from './directives'; diff --git a/src/utilities/__tests__/buildASTSchema-test.ts b/src/utilities/__tests__/buildASTSchema-test.ts index 7427ebb507..29280474ec 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'; @@ -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__/findBreakingChanges-test.ts b/src/utilities/__tests__/findBreakingChanges-test.ts index 5a7956ae5d..ba526deb48 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'; @@ -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 84f30fc0e2..85c7a4edd3 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -656,6 +656,11 @@ describe('Type System Printer', () => { url: String! ) on SCALAR + """ + Indicates an Object is a OneOf Object or an Input Object is a OneOf Input Object. + """ + directive @oneOf on OBJECT | 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. """ From 534b243baa5a3b0b101c7dba7741583a447d661a Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Fri, 14 Jan 2022 12:31:23 -0500 Subject: [PATCH 02/16] Add `__Type.oneOf` This exposes a new boolean `oneOf` field on `__Type` that indicates if a type is `oneOf`. We implement by checking if the AST node for the object contains a `@oneOf` directive. --- src/type/__tests__/introspection-test.ts | 89 +++++++++++++++++++++ src/type/definition.ts | 7 ++ src/type/introspection.ts | 10 +++ src/utilities/__tests__/printSchema-test.ts | 1 + src/utilities/extendSchema.ts | 12 +++ 5 files changed, 119 insertions(+) diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index de1344901d..0ee64385ae 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: 'oneOf', + args: [], + type: { + kind: 'SCALAR', + name: 'Boolean', + ofType: null, + }, + isDeprecated: false, + deprecationReason: null, + }, ], inputFields: null, interfaces: [], @@ -1525,6 +1536,84 @@ describe('Introspection', () => { }); }); + it('identifies oneOf for objects', () => { + const schema = buildSchema(` + type SomeObject @oneOf { + a: String + } + + type AnotherObject { + a: String + b: String + } + + type Query { + someField: String + } + `); + + const source = ` + { + a: __type(name: "SomeObject") { + oneOf + } + b: __type(name: "AnotherObject") { + oneOf + } + } + `; + + expect(graphqlSync({ schema, source })).to.deep.equal({ + data: { + a: { + oneOf: true, + }, + b: { + oneOf: false, + }, + }, + }); + }); + + 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 + } + `); + + const source = ` + { + a: __type(name: "SomeInputObject") { + oneOf + } + b: __type(name: "AnotherInputObject") { + oneOf + } + } + `; + + expect(graphqlSync({ schema, source })).to.deep.equal({ + data: { + a: { + oneOf: true, + }, + b: { + oneOf: false, + }, + }, + }); + }); + it('fails as expected on the __type root field without an arg', () => { const schema = buildSchema(` type Query { diff --git a/src/type/definition.ts b/src/type/definition.ts index 5e0c6d0472..8eab4a570c 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -763,6 +763,7 @@ export class GraphQLObjectType { extensions: Readonly>; astNode: Maybe; extensionASTNodes: ReadonlyArray; + isOneOf: boolean; private _fields: ThunkObjMap>; private _interfaces: ThunkReadonlyArray; @@ -774,6 +775,7 @@ export class GraphQLObjectType { this.extensions = toObjMap(config.extensions); this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes ?? []; + this.isOneOf = config.isOneOf ?? false; this._fields = () => defineFieldMap(config); this._interfaces = () => defineInterfaces(config); @@ -942,6 +944,7 @@ export interface GraphQLObjectTypeConfig { extensions?: Maybe>>; astNode?: Maybe; extensionASTNodes?: Maybe>; + isOneOf?: boolean; } interface GraphQLObjectTypeNormalizedConfig @@ -1611,6 +1614,7 @@ export class GraphQLInputObjectType { extensions: Readonly; astNode: Maybe; extensionASTNodes: ReadonlyArray; + isOneOf: boolean; private _fields: ThunkObjMap; @@ -1620,6 +1624,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); } @@ -1652,6 +1657,7 @@ export class GraphQLInputObjectType { extensions: this.extensions, astNode: this.astNode, extensionASTNodes: this.extensionASTNodes, + isOneOf: this.isOneOf, }; } @@ -1697,6 +1703,7 @@ export interface GraphQLInputObjectTypeConfig { extensions?: Maybe>; astNode?: Maybe; extensionASTNodes?: Maybe>; + isOneOf?: boolean; } interface GraphQLInputObjectTypeNormalizedConfig diff --git a/src/type/introspection.ts b/src/type/introspection.ts index e5fce6f241..e2e6a7b503 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -323,6 +323,16 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({ type: __Type, resolve: (type) => ('ofType' in type ? type.ofType : undefined), }, + oneOf: { + type: GraphQLBoolean, + resolve: (type) => { + if (isInputObjectType(type) || isObjectType(type)) { + return type.isOneOf; + } + + return null; + }, + }, } as GraphQLFieldConfigMap), }); diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 85c7a4edd3..22eca97483 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -703,6 +703,7 @@ describe('Type System Printer', () => { enumValues(includeDeprecated: Boolean = false): [__EnumValue!] inputFields(includeDeprecated: Boolean = false): [__InputValue!] ofType: __Type + oneOf: Boolean } """An enum describing what kind of type a given \`__Type\` is.""" diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index 998847e9f1..bf666d08c7 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -66,6 +66,7 @@ import { import { GraphQLDeprecatedDirective, GraphQLDirective, + GraphQLOneOfDirective, GraphQLSpecifiedByDirective, } from '../type/directives'; import { introspectionTypes, isIntrospectionType } from '../type/introspection'; @@ -592,6 +593,7 @@ export function extendSchemaImpl( fields: () => buildFieldMap(allNodes), astNode, extensionASTNodes, + isOneOf: isOneOf(astNode), }); } case Kind.INTERFACE_TYPE_DEFINITION: { @@ -646,6 +648,7 @@ export function extendSchemaImpl( fields: () => buildInputFieldMap(allNodes), astNode, extensionASTNodes, + isOneOf: isOneOf(astNode), }); } } @@ -682,3 +685,12 @@ 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 | ObjectTypeDefinitionNode, +): boolean { + return Boolean(getDirectiveValues(GraphQLOneOfDirective, node)); +} From 25928e3174fc342d02b58ef270b6cc3474fa2b2e Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Sat, 15 Jan 2022 10:59:45 -0500 Subject: [PATCH 03/16] Validate oneOf types This validates that all fields of oneOf objects are nullable and that all fields of oneOf input object are nullable and do not have a default value. --- src/type/__tests__/validation-test.ts | 65 ++++++++++++++++++++++++++- src/type/validate.ts | 42 +++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 629e0b8c3c..7dc6944470 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 @@ -1663,6 +1663,69 @@ describe('Type System: Input Object fields must have input types', () => { }); }); +describe('Type System: OneOf Object fields must be nullable', () => { + it('rejects non-nullable fields', () => { + const schema = buildSchema(` + type Query { + test: SomeObject + } + + type SomeObject @oneOf { + a: String + b: String! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Field SomeObject.b must be nullable as it is part of a OneOf Type.', + locations: [{ line: 8, column: 12 }], + }, + ]); + }); +}); + +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/validate.ts b/src/type/validate.ts index 92f7078757..36baa33327 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -20,6 +20,7 @@ import { isEqualType, isTypeSubTypeOf } from '../utilities/typeComparators'; import type { GraphQLEnumType, + GraphQLField, GraphQLInputField, GraphQLInputObjectType, GraphQLInterfaceType, @@ -308,6 +309,23 @@ function validateFields( ); } } + + if (isObjectType(type) && type.isOneOf) { + validateOneOfObjectField(type, field, context); + } + } +} + +function validateOneOfObjectField( + type: GraphQLObjectType, + field: GraphQLField, + context: SchemaValidationContext, +): void { + if (isNonNullType(field.type)) { + context.reportError( + `Field ${type.name}.${field.name} must be nullable as it is part of a OneOf Type.`, + field.astNode?.type, + ); } } @@ -531,6 +549,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) { + context.reportError( + `OneOf input field ${type.name}.${field.name} cannot have a default value.`, + field.astNode, + ); } } From 24f0d9528a0c7d6eccff9dd1a02ed8140ab1e402 Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Thu, 20 Jan 2022 09:44:44 -0500 Subject: [PATCH 04/16] Enforce oneOf rules during the validation phase This ensures that OneOf Input Objects have exactly one field provided with the correct non-null, type for that field. Additionally, a variable used in a OneOf Input Object must be non-nullable. --- .../__tests__/ValuesOfCorrectTypeRule-test.ts | 86 +++++++++++++++++++ src/validation/__tests__/harness.ts | 6 ++ .../rules/ValuesOfCorrectTypeRule.ts | 80 ++++++++++++++++- 3 files changed, 171 insertions(+), 1 deletion(-) diff --git a/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts b/src/validation/__tests__/ValuesOfCorrectTypeRule-test.ts index 76b03035da..3610fa648b 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 661256c56d..ea4840341c 100644 --- a/src/validation/__tests__/harness.ts +++ b/src/validation/__tests__/harness.ts @@ -79,6 +79,11 @@ export const testSchema: GraphQLSchema = buildSchema(` stringListField: [String] } + input OneOfInput @oneOf { + stringField: String + intField: Int + } + type ComplicatedArgs { # TODO List # TODO Coercion @@ -93,6 +98,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 158691c50c..34027a6bb2 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -1,14 +1,22 @@ import { didYouMean } from '../../jsutils/didYouMean'; import { inspect } from '../../jsutils/inspect'; import { keyMap } from '../../jsutils/keyMap'; +import type { ObjMap } from '../../jsutils/ObjMap'; import { suggestionList } from '../../jsutils/suggestionList'; import { GraphQLError } from '../../error/GraphQLError'; -import type { ValueNode } from '../../language/ast'; +import type { + ObjectFieldNode, + ObjectValueNode, + ValueNode, + VariableDefinitionNode, +} from '../../language/ast'; +import { Kind } from '../../language/kinds'; import { print } from '../../language/printer'; import type { ASTVisitor } from '../../language/visitor'; +import type { GraphQLInputObjectType } from '../../type/definition'; import { getNamedType, getNullableType, @@ -32,7 +40,17 @@ import type { ValidationContext } from '../ValidationContext'; 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. @@ -62,6 +80,16 @@ export function ValuesOfCorrectTypeRule( ); } } + + if (type.isOneOf) { + validateOneOfInputObject( + context, + node, + type, + fieldNodeMap, + variableDefinitions, + ); + } }, ObjectField(node) { const parentType = getNamedType(context.getParentInputType()); @@ -155,3 +183,53 @@ function isValidValueNode(context: ValidationContext, node: ValueNode): void { } } } + +function validateOneOfInputObject( + context: ValidationContext, + node: ObjectValueNode, + type: GraphQLInputObjectType, + fieldNodeMap: ObjMap, + variableDefinitions: { [key: string]: VariableDefinitionNode }, +): void { + const keys = Object.keys(fieldNodeMap); + const isNotExactlyOneField = keys.length !== 1; + + if (isNotExactlyOneField) { + context.reportError( + new GraphQLError( + `OneOf Input Object "${type.name}" must specify exactly one key.`, + node, + ), + ); + return; + } + + const value = fieldNodeMap[keys[0]].value; + const isNullLiteral = 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.`, + 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}".`, + node, + ), + ); + } + } +} From 15e39f5b10e450a3913af0a4777b21afa77fd30f Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Wed, 19 Jan 2022 12:54:40 -0500 Subject: [PATCH 05/16] Validate oneOf input objects at execution time This ensures that we enforce the rules for OneOf Input Objects at execution time. --- src/execution/__tests__/oneof-test.ts | 140 ++++++++++++++++++ .../__tests__/coerceInputValue-test.ts | 102 +++++++++++++ src/utilities/__tests__/valueFromAST-test.ts | 18 +++ src/utilities/coerceInputValue.ts | 22 +++ src/utilities/valueFromAST.ts | 12 ++ 5 files changed, 294 insertions(+) create mode 100644 src/execution/__tests__/oneof-test.ts diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts new file mode 100644 index 0000000000..9ee9ba606c --- /dev/null +++ b/src/execution/__tests__/oneof-test.ts @@ -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 { + 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.', + }, + ], + }); + }); + }); +}); diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index a73cdc6116..1278d881aa 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -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({ diff --git a/src/utilities/__tests__/valueFromAST-test.ts b/src/utilities/__tests__/valueFromAST-test.ts index a01ed95772..92fe9863f5 100644 --- a/src/utilities/__tests__/valueFromAST-test.ts +++ b/src/utilities/__tests__/valueFromAST-test.ts @@ -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); @@ -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', () => { diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 07883db85d..ba29eca03e 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -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; } diff --git a/src/utilities/valueFromAST.ts b/src/utilities/valueFromAST.ts index 4f0cee6b29..2e6cc1c613 100644 --- a/src/utilities/valueFromAST.ts +++ b/src/utilities/valueFromAST.ts @@ -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; } From 50bb96844f527498d57f640e433863fcb768872a Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Fri, 21 Jan 2022 08:58:52 -0500 Subject: [PATCH 06/16] Validate oneOf objects at execution time This ensures the OneOf Objects resolve to an object with exactly one non-null entry. --- src/execution/__tests__/oneof-test.ts | 137 +++++++++++++++++++++++++- src/execution/execute.ts | 63 +++++++++++- 2 files changed, 196 insertions(+), 4 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index 9ee9ba606c..7db4980b2a 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -37,8 +37,22 @@ function executeQuery( return execute({ schema, document: parse(query), rootValue, variableValues }); } -describe('Execute: Handles Oneof Input Objects and Oneof Objects', () => { - describe('Oneof Input Objects', () => { +async function executeQueryAsync( + query: string, + rootValue: unknown, + variableValues?: { [variable: string]: unknown }, +): Promise { + const result = await execute({ + schema, + document: parse(query), + rootValue, + variableValues, + }); + return result; +} + +describe('Execute: Handles OneOf Input Objects and OneOf Objects', () => { + describe('OneOf Input Objects', () => { const rootValue = { test({ input }: { input: { a?: string; b?: number } }) { return input; @@ -137,4 +151,123 @@ describe('Execute: Handles Oneof Input Objects and Oneof Objects', () => { }); }); }); + + describe('OneOf Objects', () => { + const query = ` + query ($input: TestInputObject! = {a: "abc"}) { + test(input: $input) { + a + b + } + } + `; + + it('works with a single, non-null value', () => { + const rootValue = { + test: { + a: null, + b: 123, + }, + }; + const result = executeQuery(query, rootValue); + + expectJSON(result).toDeepEqual({ + data: { + test: { + a: null, + b: 123, + }, + }, + }); + }); + + it('works with a single, non-null, async value', async () => { + const rootValue = { + test() { + return { + a: null, + b: () => new Promise((resolve) => resolve(123)), + }; + }, + }; + const result = await executeQueryAsync(query, rootValue); + + expectJSON(result).toDeepEqual({ + data: { + test: { + a: null, + b: 123, + }, + }, + }); + }); + + it('errors when there are no non-null values', () => { + const rootValue = { + test: { + a: null, + b: null, + }, + }; + const result = executeQuery(query, rootValue); + + expectJSON(result).toDeepEqual({ + data: { test: null }, + errors: [ + { + locations: [{ column: 11, line: 3 }], + message: + 'OneOf Object "TestObject" must have exactly one non-null field but got 0.', + path: ['test'], + }, + ], + }); + }); + + it('errors when there are multiple non-null values', () => { + const rootValue = { + test: { + a: 'abc', + b: 456, + }, + }; + const result = executeQuery(query, rootValue); + + expectJSON(result).toDeepEqual({ + data: { test: null }, + errors: [ + { + locations: [{ column: 11, line: 3 }], + message: + 'OneOf Object "TestObject" must have exactly one non-null field but got 2.', + path: ['test'], + }, + ], + }); + }); + + it('errors when there are multiple non-null, async values', async () => { + const rootValue = { + test() { + return { + a: 'abc', + b: () => new Promise((resolve) => resolve(123)), + }; + }, + }; + const result = await executeQueryAsync(query, rootValue); + + expectJSON(result).toDeepEqual({ + data: { test: null }, + errors: [ + { + locations: [{ column: 11, line: 3 }], + message: + 'OneOf Object "TestObject" must have exactly one non-null field but got 2.', + path: ['test'], + }, + ], + }); + }); + }); }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index d3c21385e8..6b43346b97 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -927,12 +927,13 @@ function completeObjectValue( if (!resolvedIsTypeOf) { throw invalidReturnTypeError(returnType, result, fieldNodes); } - return executeFields( + return executeFieldsWithOneOfValidation( exeContext, returnType, result, path, subFieldNodes, + fieldNodes, ); }); } @@ -942,7 +943,65 @@ function completeObjectValue( } } - return executeFields(exeContext, returnType, result, path, subFieldNodes); + return executeFieldsWithOneOfValidation( + exeContext, + returnType, + result, + path, + subFieldNodes, + fieldNodes, + ); +} + +function executeFieldsWithOneOfValidation( + exeContext: ExecutionContext, + parentType: GraphQLObjectType, + sourceValue: unknown, + path: Path | undefined, + fields: Map>, + fieldNodes: ReadonlyArray, +): PromiseOrValue> { + const value = executeFields( + exeContext, + parentType, + sourceValue, + path, + fields, + ); + if (!parentType.isOneOf) { + return value; + } + + if (isPromise(value)) { + return value.then((resolvedValue) => { + validateOneOfValue(resolvedValue, parentType, fieldNodes); + return resolvedValue; + }); + } + + validateOneOfValue(value, parentType, fieldNodes); + return value; +} + +function validateOneOfValue( + value: ObjMap, + returnType: GraphQLObjectType, + fieldNodes: ReadonlyArray, +): void { + let nonNullCount = 0; + + for (const field in value) { + if (value[field] !== null) { + nonNullCount += 1; + } + } + + if (nonNullCount !== 1) { + throw new GraphQLError( + `OneOf Object "${returnType.name}" must have exactly one non-null field but got ${nonNullCount}.`, + fieldNodes, + ); + } } function invalidReturnTypeError( From 36f44d36724612fba8f5d15b8ce8b6c524d3b78d Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Sat, 2 Apr 2022 09:06:10 -0400 Subject: [PATCH 07/16] Remove oneOf objects We need to more fully specify how oneOf objects should behave, so we are removing them for now. --- src/execution/__tests__/oneof-test.ts | 137 +------------------- src/execution/execute.ts | 63 +-------- src/type/__tests__/introspection-test.ts | 41 +----- src/type/__tests__/validation-test.ts | 22 ---- src/type/definition.ts | 3 - src/type/directives.ts | 7 +- src/type/introspection.ts | 2 +- src/type/validate.ts | 18 --- src/utilities/__tests__/printSchema-test.ts | 6 +- src/utilities/extendSchema.ts | 5 +- 10 files changed, 12 insertions(+), 292 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index 7db4980b2a..0dd073ca51 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -19,7 +19,7 @@ const schema = buildSchema(` b: Int } - type TestObject @oneOf { + type TestObject { a: String b: Int } @@ -37,21 +37,7 @@ function executeQuery( return execute({ schema, document: parse(query), rootValue, variableValues }); } -async function executeQueryAsync( - query: string, - rootValue: unknown, - variableValues?: { [variable: string]: unknown }, -): Promise { - const result = await execute({ - schema, - document: parse(query), - rootValue, - variableValues, - }); - return result; -} - -describe('Execute: Handles OneOf Input Objects and OneOf Objects', () => { +describe('Execute: Handles OneOf Input Objects', () => { describe('OneOf Input Objects', () => { const rootValue = { test({ input }: { input: { a?: string; b?: number } }) { @@ -151,123 +137,4 @@ describe('Execute: Handles OneOf Input Objects and OneOf Objects', () => { }); }); }); - - describe('OneOf Objects', () => { - const query = ` - query ($input: TestInputObject! = {a: "abc"}) { - test(input: $input) { - a - b - } - } - `; - - it('works with a single, non-null value', () => { - const rootValue = { - test: { - a: null, - b: 123, - }, - }; - const result = executeQuery(query, rootValue); - - expectJSON(result).toDeepEqual({ - data: { - test: { - a: null, - b: 123, - }, - }, - }); - }); - - it('works with a single, non-null, async value', async () => { - const rootValue = { - test() { - return { - a: null, - b: () => new Promise((resolve) => resolve(123)), - }; - }, - }; - const result = await executeQueryAsync(query, rootValue); - - expectJSON(result).toDeepEqual({ - data: { - test: { - a: null, - b: 123, - }, - }, - }); - }); - - it('errors when there are no non-null values', () => { - const rootValue = { - test: { - a: null, - b: null, - }, - }; - const result = executeQuery(query, rootValue); - - expectJSON(result).toDeepEqual({ - data: { test: null }, - errors: [ - { - locations: [{ column: 11, line: 3 }], - message: - 'OneOf Object "TestObject" must have exactly one non-null field but got 0.', - path: ['test'], - }, - ], - }); - }); - - it('errors when there are multiple non-null values', () => { - const rootValue = { - test: { - a: 'abc', - b: 456, - }, - }; - const result = executeQuery(query, rootValue); - - expectJSON(result).toDeepEqual({ - data: { test: null }, - errors: [ - { - locations: [{ column: 11, line: 3 }], - message: - 'OneOf Object "TestObject" must have exactly one non-null field but got 2.', - path: ['test'], - }, - ], - }); - }); - - it('errors when there are multiple non-null, async values', async () => { - const rootValue = { - test() { - return { - a: 'abc', - b: () => new Promise((resolve) => resolve(123)), - }; - }, - }; - const result = await executeQueryAsync(query, rootValue); - - expectJSON(result).toDeepEqual({ - data: { test: null }, - errors: [ - { - locations: [{ column: 11, line: 3 }], - message: - 'OneOf Object "TestObject" must have exactly one non-null field but got 2.', - path: ['test'], - }, - ], - }); - }); - }); }); diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 6b43346b97..d3c21385e8 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -927,13 +927,12 @@ function completeObjectValue( if (!resolvedIsTypeOf) { throw invalidReturnTypeError(returnType, result, fieldNodes); } - return executeFieldsWithOneOfValidation( + return executeFields( exeContext, returnType, result, path, subFieldNodes, - fieldNodes, ); }); } @@ -943,65 +942,7 @@ function completeObjectValue( } } - return executeFieldsWithOneOfValidation( - exeContext, - returnType, - result, - path, - subFieldNodes, - fieldNodes, - ); -} - -function executeFieldsWithOneOfValidation( - exeContext: ExecutionContext, - parentType: GraphQLObjectType, - sourceValue: unknown, - path: Path | undefined, - fields: Map>, - fieldNodes: ReadonlyArray, -): PromiseOrValue> { - const value = executeFields( - exeContext, - parentType, - sourceValue, - path, - fields, - ); - if (!parentType.isOneOf) { - return value; - } - - if (isPromise(value)) { - return value.then((resolvedValue) => { - validateOneOfValue(resolvedValue, parentType, fieldNodes); - return resolvedValue; - }); - } - - validateOneOfValue(value, parentType, fieldNodes); - return value; -} - -function validateOneOfValue( - value: ObjMap, - returnType: GraphQLObjectType, - fieldNodes: ReadonlyArray, -): void { - let nonNullCount = 0; - - for (const field in value) { - if (value[field] !== null) { - nonNullCount += 1; - } - } - - if (nonNullCount !== 1) { - throw new GraphQLError( - `OneOf Object "${returnType.name}" must have exactly one non-null field but got ${nonNullCount}.`, - fieldNodes, - ); - } + return executeFields(exeContext, returnType, result, path, subFieldNodes); } function invalidReturnTypeError( diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 0ee64385ae..6899468c52 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1003,7 +1003,7 @@ describe('Introspection', () => { { name: 'oneOf', isRepeatable: false, - locations: ['OBJECT', 'INPUT_OBJECT'], + locations: ['INPUT_OBJECT'], args: [], }, ], @@ -1536,45 +1536,6 @@ describe('Introspection', () => { }); }); - it('identifies oneOf for objects', () => { - const schema = buildSchema(` - type SomeObject @oneOf { - a: String - } - - type AnotherObject { - a: String - b: String - } - - type Query { - someField: String - } - `); - - const source = ` - { - a: __type(name: "SomeObject") { - oneOf - } - b: __type(name: "AnotherObject") { - oneOf - } - } - `; - - expect(graphqlSync({ schema, source })).to.deep.equal({ - data: { - a: { - oneOf: true, - }, - b: { - oneOf: false, - }, - }, - }); - }); - it('identifies oneOf for input objects', () => { const schema = buildSchema(` input SomeInputObject @oneOf { diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index 7dc6944470..c7e3a15449 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -1663,28 +1663,6 @@ describe('Type System: Input Object fields must have input types', () => { }); }); -describe('Type System: OneOf Object fields must be nullable', () => { - it('rejects non-nullable fields', () => { - const schema = buildSchema(` - type Query { - test: SomeObject - } - - type SomeObject @oneOf { - a: String - b: String! - } - `); - expectJSON(validateSchema(schema)).toDeepEqual([ - { - message: - 'Field SomeObject.b must be nullable as it is part of a OneOf Type.', - locations: [{ line: 8, column: 12 }], - }, - ]); - }); -}); - describe('Type System: OneOf Input Object fields must be nullable', () => { it('rejects non-nullable fields', () => { const schema = buildSchema(` diff --git a/src/type/definition.ts b/src/type/definition.ts index 8eab4a570c..5ee71481d8 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -763,7 +763,6 @@ export class GraphQLObjectType { extensions: Readonly>; astNode: Maybe; extensionASTNodes: ReadonlyArray; - isOneOf: boolean; private _fields: ThunkObjMap>; private _interfaces: ThunkReadonlyArray; @@ -775,7 +774,6 @@ export class GraphQLObjectType { this.extensions = toObjMap(config.extensions); this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes ?? []; - this.isOneOf = config.isOneOf ?? false; this._fields = () => defineFieldMap(config); this._interfaces = () => defineInterfaces(config); @@ -944,7 +942,6 @@ export interface GraphQLObjectTypeConfig { extensions?: Maybe>>; astNode?: Maybe; extensionASTNodes?: Maybe>; - isOneOf?: boolean; } interface GraphQLObjectTypeNormalizedConfig diff --git a/src/type/directives.ts b/src/type/directives.ts index b225d6062f..b29e9ae9c9 100644 --- a/src/type/directives.ts +++ b/src/type/directives.ts @@ -210,13 +210,12 @@ export const GraphQLSpecifiedByDirective: GraphQLDirective = }); /** - * Used to declare an Input Object as a OneOf Input Objects and an Object as a OneOf Object. + * Used to declare an Input Object as a OneOf Input Objects. */ export const GraphQLOneOfDirective: GraphQLDirective = new GraphQLDirective({ name: 'oneOf', - description: - 'Indicates an Object is a OneOf Object or an Input Object is a OneOf Input Object.', - locations: [DirectiveLocation.OBJECT, DirectiveLocation.INPUT_OBJECT], + description: 'Indicates an Input Object is a OneOf Input Object.', + locations: [DirectiveLocation.INPUT_OBJECT], args: {}, }); diff --git a/src/type/introspection.ts b/src/type/introspection.ts index e2e6a7b503..8e1354c2a1 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -326,7 +326,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({ oneOf: { type: GraphQLBoolean, resolve: (type) => { - if (isInputObjectType(type) || isObjectType(type)) { + if (isInputObjectType(type)) { return type.isOneOf; } diff --git a/src/type/validate.ts b/src/type/validate.ts index 36baa33327..16d2896aaa 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -20,7 +20,6 @@ import { isEqualType, isTypeSubTypeOf } from '../utilities/typeComparators'; import type { GraphQLEnumType, - GraphQLField, GraphQLInputField, GraphQLInputObjectType, GraphQLInterfaceType, @@ -309,23 +308,6 @@ function validateFields( ); } } - - if (isObjectType(type) && type.isOneOf) { - validateOneOfObjectField(type, field, context); - } - } -} - -function validateOneOfObjectField( - type: GraphQLObjectType, - field: GraphQLField, - context: SchemaValidationContext, -): void { - if (isNonNullType(field.type)) { - context.reportError( - `Field ${type.name}.${field.name} must be nullable as it is part of a OneOf Type.`, - field.astNode?.type, - ); } } diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 22eca97483..b61fbd6d2c 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -656,10 +656,8 @@ describe('Type System Printer', () => { url: String! ) on SCALAR - """ - Indicates an Object is a OneOf Object or an Input Object is a OneOf Input Object. - """ - directive @oneOf on OBJECT | INPUT_OBJECT + """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. diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index bf666d08c7..d53752d919 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -593,7 +593,6 @@ export function extendSchemaImpl( fields: () => buildFieldMap(allNodes), astNode, extensionASTNodes, - isOneOf: isOneOf(astNode), }); } case Kind.INTERFACE_TYPE_DEFINITION: { @@ -689,8 +688,6 @@ function getSpecifiedByURL( /** * Given an input object node, returns if the node should be OneOf. */ -function isOneOf( - node: InputObjectTypeDefinitionNode | ObjectTypeDefinitionNode, -): boolean { +function isOneOf(node: InputObjectTypeDefinitionNode): boolean { return Boolean(getDirectiveValues(GraphQLOneOfDirective, node)); } From 6f7f8d56fadc2b549089032e8cbf6c51fbd3cbf5 Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Fri, 8 Apr 2022 11:27:51 -0400 Subject: [PATCH 08/16] Add test coverage --- src/execution/__tests__/oneof-test.ts | 49 +++++++++++++++++++- src/type/__tests__/introspection-test.ts | 1 + src/utilities/__tests__/valueFromAST-test.ts | 6 +++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index 0dd073ca51..27744fe1f1 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -113,7 +113,30 @@ describe('Execute: Handles OneOf Input Objects', () => { }); }); - it('rejects a bad variable', () => { + 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) { @@ -136,5 +159,29 @@ describe('Execute: Handles OneOf Input Objects', () => { ], }); }); + + 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.', + }, + ], + }); + }); }); }); diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 6899468c52..af4d9de9fd 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1549,6 +1549,7 @@ describe('Introspection', () => { type Query { someField(someArg: SomeInputObject): String + anotherField(anotherArg: AnotherInputObject): String } `); diff --git a/src/utilities/__tests__/valueFromAST-test.ts b/src/utilities/__tests__/valueFromAST-test.ts index 92fe9863f5..05924f3c56 100644 --- a/src/utilities/__tests__/valueFromAST-test.ts +++ b/src/utilities/__tests__/valueFromAST-test.ts @@ -232,6 +232,12 @@ describe('valueFromAST', () => { 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( From ac4bb46efae4b875c623ab784506bd197c993ca1 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Mon, 23 May 2022 13:57:55 +0100 Subject: [PATCH 09/16] Add another test to increase coverage --- src/type/__tests__/introspection-test.ts | 59 ++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index af4d9de9fd..4c786402c9 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1576,6 +1576,65 @@ describe('Introspection', () => { }); }); + it('returns null for oneOf for other types', () => { + const schema = buildSchema(` + type A implements I { + a: String + } + enum E { + A + } + interface I { + a: String + } + union U = A + type Query { + someField(e: E): U + anotherField(e: E): I + } + `); + + const source = ` + { + a: __type(name: "A") { + oneOf + } + e: __type(name: "E") { + oneOf + } + i: __type(name: "I") { + oneOf + } + o: __type(name: "String") { + oneOf + } + u: __type(name: "U") { + oneOf + } + } + `; + + expect(graphqlSync({ schema, source })).to.deep.equal({ + data: { + a: { + oneOf: null, + }, + e: { + oneOf: null, + }, + i: { + oneOf: null, + }, + o: { + oneOf: null, + }, + u: { + oneOf: null, + }, + }, + }); + }); + it('fails as expected on the __type root field without an arg', () => { const schema = buildSchema(` type Query { From a081f850c5fafb4fa5f5f68d53bbc094b6e7ea29 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Mon, 23 May 2022 15:30:17 +0100 Subject: [PATCH 10/16] Upgrade GraphQLError calls --- src/validation/rules/ValuesOfCorrectTypeRule.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 4a3e6af52b..79d0b93e3c 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -194,7 +194,7 @@ function validateOneOfInputObject( context.reportError( new GraphQLError( `OneOf Input Object "${type.name}" must specify exactly one key.`, - node, + { nodes: [node] }, ), ); return; @@ -206,10 +206,9 @@ function validateOneOfInputObject( if (isNullLiteral) { context.reportError( - new GraphQLError( - `Field "${type.name}.${keys[0]}" must be non-null.`, - node, - ), + new GraphQLError(`Field "${type.name}.${keys[0]}" must be non-null.`, { + nodes: [node], + }), ); return; } @@ -223,7 +222,7 @@ function validateOneOfInputObject( context.reportError( new GraphQLError( `Variable "${variableName}" must be non-nullable to be used for OneOf Input Object "${type.name}".`, - node, + { nodes: [node] }, ), ); } From dc584765f224c7a753418ddfbf96a7d7b3d8a9bc Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 25 May 2022 10:44:20 +0100 Subject: [PATCH 11/16] Rename __Type.oneOf to __Type.isOneOf --- src/type/__tests__/introspection-test.ts | 32 ++++++++++----------- src/type/introspection.ts | 2 +- src/utilities/__tests__/printSchema-test.ts | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index 3220de1b5b..c6b1b7e7e1 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -373,7 +373,7 @@ describe('Introspection', () => { deprecationReason: null, }, { - name: 'oneOf', + name: 'isOneOf', args: [], type: { kind: 'SCALAR', @@ -1001,7 +1001,7 @@ describe('Introspection', () => { ], }, { - name: 'oneOf', + name: 'isOneOf', isRepeatable: false, locations: ['INPUT_OBJECT'], args: [], @@ -1556,10 +1556,10 @@ describe('Introspection', () => { const source = ` { a: __type(name: "SomeInputObject") { - oneOf + isOneOf } b: __type(name: "AnotherInputObject") { - oneOf + isOneOf } } `; @@ -1567,10 +1567,10 @@ describe('Introspection', () => { expect(graphqlSync({ schema, source })).to.deep.equal({ data: { a: { - oneOf: true, + isOneOf: true, }, b: { - oneOf: false, + isOneOf: false, }, }, }); @@ -1597,19 +1597,19 @@ describe('Introspection', () => { const source = ` { a: __type(name: "A") { - oneOf + isOneOf } e: __type(name: "E") { - oneOf + isOneOf } i: __type(name: "I") { - oneOf + isOneOf } o: __type(name: "String") { - oneOf + isOneOf } u: __type(name: "U") { - oneOf + isOneOf } } `; @@ -1617,19 +1617,19 @@ describe('Introspection', () => { expect(graphqlSync({ schema, source })).to.deep.equal({ data: { a: { - oneOf: null, + isOneOf: null, }, e: { - oneOf: null, + isOneOf: null, }, i: { - oneOf: null, + isOneOf: null, }, o: { - oneOf: null, + isOneOf: null, }, u: { - oneOf: null, + isOneOf: null, }, }, }); diff --git a/src/type/introspection.ts b/src/type/introspection.ts index 8e1354c2a1..ef8b851714 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -323,7 +323,7 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({ type: __Type, resolve: (type) => ('ofType' in type ? type.ofType : undefined), }, - oneOf: { + isOneOf: { type: GraphQLBoolean, resolve: (type) => { if (isInputObjectType(type)) { diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 91e0ce6490..d077f8fc07 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -717,7 +717,7 @@ describe('Type System Printer', () => { enumValues(includeDeprecated: Boolean = false): [__EnumValue!] inputFields(includeDeprecated: Boolean = false): [__InputValue!] ofType: __Type - oneOf: Boolean + isOneOf: Boolean } """An enum describing what kind of type a given \`__Type\` is.""" From ec88066aec87f004643d19225c27aab97fec98fc Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 25 May 2022 10:44:29 +0100 Subject: [PATCH 12/16] Remove unnecessary return --- src/type/introspection.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/type/introspection.ts b/src/type/introspection.ts index ef8b851714..e876169645 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -329,8 +329,6 @@ export const __Type: GraphQLObjectType = new GraphQLObjectType({ if (isInputObjectType(type)) { return type.isOneOf; } - - return null; }, }, } as GraphQLFieldConfigMap), From 77fb4b9e5bf69cbc66660f64e59ba54f69ac865a Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 25 May 2022 10:48:26 +0100 Subject: [PATCH 13/16] Oops --- src/type/__tests__/introspection-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/type/__tests__/introspection-test.ts b/src/type/__tests__/introspection-test.ts index c6b1b7e7e1..fcc42b54cb 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1001,7 +1001,7 @@ describe('Introspection', () => { ], }, { - name: 'isOneOf', + name: 'oneOf', isRepeatable: false, locations: ['INPUT_OBJECT'], args: [], From e9302ae7f23fad09861521ccf7759ec20ba3807e Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Wed, 22 Feb 2023 08:18:36 +0000 Subject: [PATCH 14/16] Lint --- src/execution/__tests__/oneof-test.ts | 10 +++++----- src/validation/rules/ValuesOfCorrectTypeRule.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index 27744fe1f1..96666cef2e 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -1,13 +1,13 @@ import { describe, it } from 'mocha'; -import { expectJSON } from '../../__testUtils__/expectJSON'; +import { expectJSON } from '../../__testUtils__/expectJSON.js'; -import { parse } from '../../language/parser'; +import { parse } from '../../language/parser.js'; -import { buildSchema } from '../../utilities/buildASTSchema'; +import { buildSchema } from '../../utilities/buildASTSchema.js'; -import type { ExecutionResult } from '../execute'; -import { execute } from '../execute'; +import type { ExecutionResult } from '../execute.js'; +import { execute } from '../execute.js'; const schema = buildSchema(` type Query { diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index 312a3eca09..e379b5b7f7 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -16,7 +16,7 @@ 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'; +import type { GraphQLInputObjectType } from '../../type/definition.js'; import { getNamedType, getNullableType, From 84a87ee934dfedf5a7f2e66cc57ce6401eb37e48 Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Fri, 19 May 2023 08:35:18 -0400 Subject: [PATCH 15/16] Fix lint violations --- src/type/validate.ts | 2 +- src/validation/rules/ValuesOfCorrectTypeRule.ts | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/type/validate.ts b/src/type/validate.ts index 23ced20e38..deee23a372 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -561,7 +561,7 @@ function validateOneOfInputObjectField( ); } - if (field.defaultValue) { + if (field.defaultValue !== undefined) { context.reportError( `OneOf input field ${type.name}.${field.name} cannot have a default value.`, field.astNode, diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index a60ac8def5..3b9a1ee645 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -1,7 +1,5 @@ import { didYouMean } from '../../jsutils/didYouMean.js'; import { inspect } from '../../jsutils/inspect.js'; -import { keyMap } from '../../jsutils/keyMap.js'; -import type { ObjMap } from '../../jsutils/ObjMap.js'; import { suggestionList } from '../../jsutils/suggestionList.js'; import { GraphQLError } from '../../error/GraphQLError.js'; @@ -186,10 +184,10 @@ function validateOneOfInputObject( context: ValidationContext, node: ObjectValueNode, type: GraphQLInputObjectType, - fieldNodeMap: ObjMap, + fieldNodeMap: Map, variableDefinitions: { [key: string]: VariableDefinitionNode }, ): void { - const keys = Object.keys(fieldNodeMap); + const keys = Array.from(fieldNodeMap.keys()); const isNotExactlyOneField = keys.length !== 1; if (isNotExactlyOneField) { @@ -202,9 +200,9 @@ function validateOneOfInputObject( return; } - const value = fieldNodeMap[keys[0]].value; - const isNullLiteral = value.kind === Kind.NULL; - const isVariable = value.kind === Kind.VARIABLE; + const value = fieldNodeMap.get(keys[0])?.value; + const isNullLiteral = !value || value.kind === Kind.NULL; + const isVariable = value?.kind === Kind.VARIABLE; if (isNullLiteral) { context.reportError( From cfc68cdea06ed73c55ed10ef4a816fa1658e8e1a Mon Sep 17 00:00:00 2001 From: Erik Kessler Date: Fri, 19 May 2023 09:38:34 -0400 Subject: [PATCH 16/16] Address PR feedback --- src/__testUtils__/kitchenSinkSDL.ts | 6 ++ src/execution/__tests__/oneof-test.ts | 10 ++-- src/language/__tests__/schema-printer-test.ts | 6 ++ src/type/__tests__/introspection-test.ts | 56 ++++++++----------- .../__tests__/coerceInputValue-test.ts | 9 ++- src/utilities/coerceInputValue.ts | 4 +- 6 files changed, 48 insertions(+), 43 deletions(-) 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 index 96666cef2e..acde6031b4 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -23,10 +23,6 @@ const schema = buildSchema(` a: String b: Int } - - schema { - query: Query - } `); function executeQuery( @@ -85,6 +81,8 @@ describe('Execute: Handles OneOf Input Objects', () => { { 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'], }, @@ -154,7 +152,7 @@ describe('Execute: Handles OneOf Input Objects', () => { { locations: [{ column: 16, line: 2 }], message: - 'Variable "$input" got invalid value { a: "abc", b: 123 }; Exactly one key must be specified.', + 'Variable "$input" got invalid value { a: "abc", b: 123 }; Exactly one key must be specified for OneOf type "TestInputObject".', }, ], }); @@ -178,7 +176,7 @@ describe('Execute: Handles OneOf Input Objects', () => { { locations: [{ column: 16, line: 2 }], message: - 'Variable "$input" got invalid value { a: "abc", b: null }; Exactly one key must be specified.', + 'Variable "$input" got invalid value { a: "abc", b: null }; Exactly one key must be specified for OneOf type "TestInputObject".', }, ], }); 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 beef56fd46..e6e5d0943e 100644 --- a/src/type/__tests__/introspection-test.ts +++ b/src/type/__tests__/introspection-test.ts @@ -1555,10 +1555,10 @@ describe('Introspection', () => { const source = ` { - a: __type(name: "SomeInputObject") { + oneOfInputObject: __type(name: "SomeInputObject") { isOneOf } - b: __type(name: "AnotherInputObject") { + inputObject: __type(name: "AnotherInputObject") { isOneOf } } @@ -1566,10 +1566,10 @@ describe('Introspection', () => { expect(graphqlSync({ schema, source })).to.deep.equal({ data: { - a: { + oneOfInputObject: { isOneOf: true, }, - b: { + inputObject: { isOneOf: false, }, }, @@ -1578,37 +1578,37 @@ describe('Introspection', () => { it('returns null for oneOf for other types', () => { const schema = buildSchema(` - type A implements I { - a: String + type SomeObject implements SomeInterface { + fieldA: String } - enum E { - A + enum SomeEnum { + SomeObject } - interface I { - a: String + interface SomeInterface { + fieldA: String } - union U = A + union SomeUnion = SomeObject type Query { - someField(e: E): U - anotherField(e: E): I + someField(enum: SomeEnum): SomeUnion + anotherField(enum: SomeEnum): SomeInterface } `); const source = ` { - a: __type(name: "A") { + object: __type(name: "SomeObject") { isOneOf } - e: __type(name: "E") { + enum: __type(name: "SomeEnum") { isOneOf } - i: __type(name: "I") { + interface: __type(name: "SomeInterface") { isOneOf } - o: __type(name: "String") { + scalar: __type(name: "String") { isOneOf } - u: __type(name: "U") { + union: __type(name: "SomeUnion") { isOneOf } } @@ -1616,21 +1616,11 @@ describe('Introspection', () => { expect(graphqlSync({ schema, source })).to.deep.equal({ data: { - a: { - isOneOf: null, - }, - e: { - isOneOf: null, - }, - i: { - isOneOf: null, - }, - o: { - isOneOf: null, - }, - u: { - isOneOf: null, - }, + object: { isOneOf: null }, + enum: { isOneOf: null }, + interface: { isOneOf: null }, + scalar: { isOneOf: null }, + union: { isOneOf: null }, }, }); }); diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index b871811259..7712c91a39 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -292,7 +292,8 @@ describe('coerceInputValue', () => { const result = coerceValue({ foo: 123, bar: null }, TestInputObject); expectErrors(result).to.deep.equal([ { - error: 'Exactly one key must be specified.', + error: + 'Exactly one key must be specified for OneOf type "TestInputObject".', path: [], value: { foo: 123, bar: null }, }, @@ -335,7 +336,8 @@ describe('coerceInputValue', () => { value: 'def', }, { - error: 'Exactly one key must be specified.', + error: + 'Exactly one key must be specified for OneOf type "TestInputObject".', path: [], value: { foo: 'abc', bar: 'def' }, }, @@ -367,7 +369,8 @@ describe('coerceInputValue', () => { value: { bart: 123 }, }, { - error: 'Exactly one key must be specified.', + error: + 'Exactly one key must be specified for OneOf type "TestInputObject".', path: [], value: { bart: 123 }, }, diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 8bbe3a5075..9aa49abed5 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -149,7 +149,9 @@ function coerceInputValueImpl( onError( pathToArray(path), inputValue, - new GraphQLError('Exactly one key must be specified.'), + new GraphQLError( + `Exactly one key must be specified for OneOf type "${type.name}".`, + ), ); }