From 1235a0d577197c4770056f4922feec0c5d646907 Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Wed, 2 Jul 2025 16:34:33 -0600 Subject: [PATCH 01/10] - Implement the oneOf directive tests, following the graphql-js implementation --- tests/Type/IntrospectionTest.php | 19 +++ tests/Type/OneOfInputObjectTest.php | 194 ++++++++++++++++++++++ tests/Utils/BreakingChangesFinderTest.php | 5 + tests/Utils/BuildSchemaTest.php | 13 +- tests/Utils/SchemaPrinterTest.php | 4 + 5 files changed, 230 insertions(+), 5 deletions(-) create mode 100644 tests/Type/OneOfInputObjectTest.php diff --git a/tests/Type/IntrospectionTest.php b/tests/Type/IntrospectionTest.php index c484db3a5..88ce63c35 100644 --- a/tests/Type/IntrospectionTest.php +++ b/tests/Type/IntrospectionTest.php @@ -365,6 +365,17 @@ public function testExecutesAnIntrospectionQuery(): void 'isDeprecated' => false, 'deprecationReason' => null, ], + 9 => [ + 'name' => 'isOneOf', + 'args' => [], + 'type' => [ + 'kind' => 'SCALAR', + 'name' => 'Boolean', + 'ofType' => null, + ], + 'isDeprecated' => false, + 'deprecationReason' => null, + ], ], 'inputFields' => null, 'interfaces' => [], @@ -962,6 +973,14 @@ public function testExecutesAnIntrospectionQuery(): void 3 => 'INPUT_FIELD_DEFINITION', ], ], + [ + 'name' => 'oneOf', + 'args' => [], + 'isRepeatable' => false, + 'locations' => [ + 0 => 'INPUT_OBJECT', + ], + ], ], ], ], diff --git a/tests/Type/OneOfInputObjectTest.php b/tests/Type/OneOfInputObjectTest.php new file mode 100644 index 000000000..e80bffac7 --- /dev/null +++ b/tests/Type/OneOfInputObjectTest.php @@ -0,0 +1,194 @@ + 'OneOfInput', + 'isOneOf' => true, + 'fields' => [ + 'stringField' => Type::string(), + 'intField' => Type::int(), + ], + ]); + + self::assertTrue($oneOfInput->isOneOf()); + self::assertCount(2, $oneOfInput->getFields()); + } + + public function testOneOfInputObjectValidation(): void + { + $oneOfInput = new InputObjectType([ + 'name' => 'OneOfInput', + 'isOneOf' => true, + 'fields' => [ + 'stringField' => Type::string(), + 'intField' => Type::int(), + ], + ]); + + // Should not throw for valid oneOf input + $oneOfInput->assertValid(); + self::addToAssertionCount(1); // Test passes if no exception thrown + } + + public function testOneOfInputObjectRejectsNonNullFields(): void + { + $this->expectException(InvariantViolation::class); + $this->expectExceptionMessage('OneOf input object type OneOfInput field stringField must be nullable'); + + $oneOfInput = new InputObjectType([ + 'name' => 'OneOfInput', + 'isOneOf' => true, + 'fields' => [ + 'stringField' => Type::nonNull(Type::string()), // This should fail + 'intField' => Type::int(), + ], + ]); + + $oneOfInput->assertValid(); + } + + public function testOneOfInputObjectRejectsDefaultValues(): void + { + $this->expectException(InvariantViolation::class); + $this->expectExceptionMessage('OneOf input object type OneOfInput field stringField cannot have a default value'); + + $oneOfInput = new InputObjectType([ + 'name' => 'OneOfInput', + 'isOneOf' => true, + 'fields' => [ + 'stringField' => [ + 'type' => Type::string(), + 'defaultValue' => 'default', // This should fail + ], + 'intField' => Type::int(), + ], + ]); + + $oneOfInput->assertValid(); + } + + public function testOneOfInputObjectSchemaValidation(): void + { + $oneOfInput = new InputObjectType([ + 'name' => 'OneOfInput', + 'isOneOf' => true, + 'fields' => [ + 'stringField' => Type::string(), + 'intField' => Type::int(), + ], + ]); + + $query = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'test' => [ + 'type' => Type::string(), + 'args' => [ + 'input' => $oneOfInput, + ], + 'resolve' => static fn ($source, $args) => 'test', + ], + ], + ]); + + $schema = new Schema(['query' => $query]); + + // Valid query with exactly one field + $validQuery = '{ test(input: { stringField: "hello" }) }'; + $result = GraphQL::executeQuery($schema, $validQuery); + self::assertEmpty($result->errors ?? []); + + // Invalid query with multiple fields + $invalidQuery = '{ test(input: { stringField: "hello", intField: 42 }) }'; + $result = GraphQL::executeQuery($schema, $invalidQuery); + self::assertNotEmpty($result->errors ?? []); + self::assertCount(1, $result->errors); + self::assertStringContainsString('must specify exactly one field', $result->errors[0]->getMessage()); + + // Invalid query with no fields + $emptyQuery = '{ test(input: {}) }'; + $result = GraphQL::executeQuery($schema, $emptyQuery); + self::assertNotEmpty($result->errors ?? []); + self::assertCount(1, $result->errors); + self::assertStringContainsString('must specify exactly one field', $result->errors[0]->getMessage()); + } + + public function testOneOfIntrospection(): void + { + $oneOfInput = new InputObjectType([ + 'name' => 'OneOfInput', + 'isOneOf' => true, + 'fields' => [ + 'stringField' => Type::string(), + 'intField' => Type::int(), + ], + ]); + + $regularInput = new InputObjectType([ + 'name' => 'RegularInput', + 'fields' => [ + 'stringField' => Type::string(), + 'intField' => Type::int(), + ], + ]); + + $query = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'test' => [ + 'type' => Type::string(), + 'resolve' => static fn () => 'test', + ], + ], + ]); + + $schema = new Schema([ + 'query' => $query, + 'types' => [$oneOfInput, $regularInput], + ]); + + $introspectionQuery = ' + { + __schema { + types { + name + isOneOf + } + } + } + '; + + $result = GraphQL::executeQuery($schema, $introspectionQuery); + self::assertEmpty($result->errors ?? []); + + $types = $result->data['__schema']['types'] ?? []; + $oneOfType = null; + $regularType = null; + + foreach ($types as $type) { + if ($type['name'] === 'OneOfInput') { + $oneOfType = $type; + } elseif ($type['name'] === 'RegularInput') { + $regularType = $type; + } + } + + self::assertNotNull($oneOfType); + self::assertNotNull($regularType); + self::assertTrue($oneOfType['isOneOf']); + self::assertFalse($regularType['isOneOf']); // Should be false for regular input objects + } +} diff --git a/tests/Utils/BreakingChangesFinderTest.php b/tests/Utils/BreakingChangesFinderTest.php index b9c365063..2f4a6c760 100644 --- a/tests/Utils/BreakingChangesFinderTest.php +++ b/tests/Utils/BreakingChangesFinderTest.php @@ -1333,6 +1333,7 @@ public function testShouldDetectIfADirectiveWasImplicitlyRemoved(): void ]); $deprecatedDirective = Directive::deprecatedDirective(); + $oneOfDirective = Directive::oneOfDirective(); self::assertEquals( [ @@ -1340,6 +1341,10 @@ public function testShouldDetectIfADirectiveWasImplicitlyRemoved(): void 'type' => BreakingChangesFinder::BREAKING_CHANGE_DIRECTIVE_REMOVED, 'description' => "{$deprecatedDirective->name} was removed", ], + [ + 'type' => BreakingChangesFinder::BREAKING_CHANGE_DIRECTIVE_REMOVED, + 'description' => "{$oneOfDirective->name} was removed", + ], ], BreakingChangesFinder::findRemovedDirectives($oldSchema, $newSchema) ); diff --git a/tests/Utils/BuildSchemaTest.php b/tests/Utils/BuildSchemaTest.php index b222f88b5..11cd8c2d8 100644 --- a/tests/Utils/BuildSchemaTest.php +++ b/tests/Utils/BuildSchemaTest.php @@ -272,11 +272,12 @@ public function testMaintainsIncludeSkipAndSpecifiedBy(): void { $schema = BuildSchema::buildAST(Parser::parse('type Query')); - // TODO switch to 4 when adding @specifiedBy - see https://github.com/webonyx/graphql-php/issues/1140 - self::assertCount(3, $schema->getDirectives()); + // TODO switch to 5 when adding @specifiedBy - see https://github.com/webonyx/graphql-php/issues/1140 + self::assertCount(4, $schema->getDirectives()); self::assertSame(Directive::skipDirective(), $schema->getDirective('skip')); self::assertSame(Directive::includeDirective(), $schema->getDirective('include')); self::assertSame(Directive::deprecatedDirective(), $schema->getDirective('deprecated')); + self::assertSame(Directive::oneOfDirective(), $schema->getDirective('oneOf')); self::markTestIncomplete('See https://github.com/webonyx/graphql-php/issues/1140'); self::assertSame(Directive::specifiedByDirective(), $schema->getDirective('specifiedBy')); @@ -292,10 +293,11 @@ public function testOverridingDirectivesExcludesSpecified(): void directive @specifiedBy on FIELD_DEFINITION ')); - self::assertCount(4, $schema->getDirectives()); + self::assertCount(5, $schema->getDirectives()); self::assertNotEquals(Directive::skipDirective(), $schema->getDirective('skip')); self::assertNotEquals(Directive::includeDirective(), $schema->getDirective('include')); self::assertNotEquals(Directive::deprecatedDirective(), $schema->getDirective('deprecated')); + self::assertSame(Directive::oneOfDirective(), $schema->getDirective('oneOf')); self::markTestIncomplete('See https://github.com/webonyx/graphql-php/issues/1140'); self::assertNotEquals(Directive::specifiedByDirective(), $schema->getDirective('specifiedBy')); @@ -310,12 +312,13 @@ public function testAddingDirectivesMaintainsIncludeSkipAndSpecifiedBy(): void GRAPHQL; $schema = BuildSchema::buildAST(Parser::parse($sdl)); - // TODO switch to 5 when adding @specifiedBy - see https://github.com/webonyx/graphql-php/issues/1140 - self::assertCount(4, $schema->getDirectives()); + // TODO switch to 6 when adding @specifiedBy - see https://github.com/webonyx/graphql-php/issues/1140 + self::assertCount(5, $schema->getDirectives()); self::assertNotNull($schema->getDirective('foo')); self::assertNotNull($schema->getDirective('skip')); self::assertNotNull($schema->getDirective('include')); self::assertNotNull($schema->getDirective('deprecated')); + self::assertNotNull($schema->getDirective('oneOf')); self::markTestIncomplete('See https://github.com/webonyx/graphql-php/issues/1140'); self::assertNotNull($schema->getDirective('specifiedBy')); diff --git a/tests/Utils/SchemaPrinterTest.php b/tests/Utils/SchemaPrinterTest.php index 797157539..a5f753c04 100644 --- a/tests/Utils/SchemaPrinterTest.php +++ b/tests/Utils/SchemaPrinterTest.php @@ -1011,6 +1011,9 @@ public function testPrintIntrospectionSchema(): void reason: String = "No longer supported" ) on FIELD_DEFINITION | ENUM_VALUE | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION + "Indicates that an input object is a oneof input object and exactly one of the input fields must be specified." + 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." type __Schema { "A list of all types supported by this server." @@ -1044,6 +1047,7 @@ interfaces: [__Type!] 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." From 982c3aaa0ab8c4f857de33b52fd5ae3aa39afeb3 Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Wed, 2 Jul 2025 16:40:16 -0600 Subject: [PATCH 02/10] - update Introspection to include isOneOf field - add oneOfDirective definition, allowed on INPUT_OBJECT - add isOneOf to InputObjectType config - update BuldSchema to account for oneOf directive - add OneOfInputObjectsRule to validation rules --- src/Type/Definition/Directive.php | 15 +++++ src/Type/Definition/InputObjectType.php | 48 ++++++++++++++ src/Type/Introspection.php | 6 ++ src/Utils/BuildSchema.php | 3 + src/Validator/DocumentValidator.php | 2 + src/Validator/Rules/OneOfInputObjectsRule.php | 65 +++++++++++++++++++ 6 files changed, 139 insertions(+) create mode 100644 src/Validator/Rules/OneOfInputObjectsRule.php diff --git a/src/Type/Definition/Directive.php b/src/Type/Definition/Directive.php index 5442942ce..f279998f5 100644 --- a/src/Type/Definition/Directive.php +++ b/src/Type/Definition/Directive.php @@ -27,6 +27,7 @@ class Directive public const SKIP_NAME = 'skip'; public const DEPRECATED_NAME = 'deprecated'; public const REASON_ARGUMENT_NAME = 'reason'; + public const ONE_OF_NAME = 'oneOf'; /** * Lazily initialized. @@ -86,6 +87,7 @@ public static function getInternalDirectives(): array self::INCLUDE_NAME => self::includeDirective(), self::SKIP_NAME => self::skipDirective(), self::DEPRECATED_NAME => self::deprecatedDirective(), + self::ONE_OF_NAME => self::oneOfDirective(), ]; } @@ -151,6 +153,19 @@ public static function deprecatedDirective(): Directive ]); } + /** @throws InvariantViolation */ + public static function oneOfDirective(): Directive + { + return self::$internalDirectives[self::ONE_OF_NAME] ??= new self([ + 'name' => self::ONE_OF_NAME, + 'description' => 'Indicates that an input object is a oneof input object and exactly one of the input fields must be specified.', + 'locations' => [ + DirectiveLocation::INPUT_OBJECT, + ], + 'args' => [], + ]); + } + /** @throws InvariantViolation */ public static function isSpecifiedDirective(Directive $directive): bool { diff --git a/src/Type/Definition/InputObjectType.php b/src/Type/Definition/InputObjectType.php index 3e9c1ad9e..66481edee 100644 --- a/src/Type/Definition/InputObjectType.php +++ b/src/Type/Definition/InputObjectType.php @@ -18,6 +18,7 @@ * name?: string|null, * description?: string|null, * fields: iterable|callable(): iterable, + * isOneOf?: bool|null, * parseValue?: callable(array): mixed, * astNode?: InputObjectTypeDefinitionNode|null, * extensionASTNodes?: array|null @@ -35,6 +36,8 @@ class InputObjectType extends Type implements InputType, NullableType, NamedType /** @phpstan-var InputObjectConfig */ public array $config; + public bool $isOneOf; + /** * Lazily initialized. * @@ -55,6 +58,7 @@ public function __construct(array $config) $this->description = $config['description'] ?? null; $this->astNode = $config['astNode'] ?? null; $this->extensionASTNodes = $config['extensionASTNodes'] ?? []; + $this->isOneOf = $config['isOneOf'] ?? false; $this->config = $config; } @@ -91,6 +95,12 @@ public function hasField(string $name): bool return isset($this->fields[$name]); } + /** Returns true if this is a oneOf input object type. */ + public function isOneOf(): bool + { + return $this->isOneOf; + } + /** * @throws InvariantViolation * @@ -196,6 +206,44 @@ public function assertValid(): void foreach ($resolvedFields as $field) { $field->assertValid($this); } + + // Additional validation for oneOf input objects + if ($this->isOneOf()) { + $this->validateOneOfConstraints($resolvedFields); + } + } + + /** + * Validates that oneOf input object constraints are met. + * + * @param array $fields + * + * @throws InvariantViolation + */ + private function validateOneOfConstraints(array $fields): void + { + if (count($fields) === 0) { + throw new InvariantViolation("OneOf input object type {$this->name} must define one or more fields."); + } + + foreach ($fields as $fieldName => $field) { + $fieldType = $field->getType(); + + // OneOf fields must be nullable (not wrapped in NonNull) + if ($fieldType instanceof NonNull) { + throw new InvariantViolation("OneOf input object type {$this->name} field {$fieldName} must be nullable."); + } + + // OneOf fields cannot have default values + if ($field->defaultValueExists()) { + throw new InvariantViolation("OneOf input object type {$this->name} field {$fieldName} cannot have a default value."); + } + + // OneOf fields cannot be deprecated (optional constraint for now) + if ($field->isDeprecated()) { + throw new InvariantViolation("OneOf input object type {$this->name} field {$fieldName} cannot be deprecated."); + } + } } public function astNode(): ?InputObjectTypeDefinitionNode diff --git a/src/Type/Introspection.php b/src/Type/Introspection.php index 0fb6e6d73..9b292252a 100644 --- a/src/Type/Introspection.php +++ b/src/Type/Introspection.php @@ -437,6 +437,12 @@ public static function _type(): ObjectType ? $type->getWrappedType() : null, ], + 'isOneOf' => [ + 'type' => Type::boolean(), + 'resolve' => static fn ($type): ?bool => $type instanceof InputObjectType + ? $type->isOneOf() + : null, + ], ], ]); } diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index 83d4d1961..349a25d14 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -238,6 +238,9 @@ static function (string $typeName): Type { if (! isset($directivesByName['deprecated'])) { $directives[] = Directive::deprecatedDirective(); } + if (! isset($directivesByName['oneOf'])) { + $directives[] = Directive::oneOfDirective(); + } // Note: While this could make early assertions to get the correctly // typed values below, that would throw immediately while type system diff --git a/src/Validator/DocumentValidator.php b/src/Validator/DocumentValidator.php index 08c4068a3..0c4eb498f 100644 --- a/src/Validator/DocumentValidator.php +++ b/src/Validator/DocumentValidator.php @@ -22,6 +22,7 @@ use GraphQL\Validator\Rules\NoUndefinedVariables; use GraphQL\Validator\Rules\NoUnusedFragments; use GraphQL\Validator\Rules\NoUnusedVariables; +use GraphQL\Validator\Rules\OneOfInputObjectsRule; use GraphQL\Validator\Rules\OverlappingFieldsCanBeMerged; use GraphQL\Validator\Rules\PossibleFragmentSpreads; use GraphQL\Validator\Rules\PossibleTypeExtensions; @@ -179,6 +180,7 @@ public static function defaultRules(): array VariablesInAllowedPosition::class => new VariablesInAllowedPosition(), OverlappingFieldsCanBeMerged::class => new OverlappingFieldsCanBeMerged(), UniqueInputFieldNames::class => new UniqueInputFieldNames(), + OneOfInputObjectsRule::class => new OneOfInputObjectsRule(), ]; } diff --git a/src/Validator/Rules/OneOfInputObjectsRule.php b/src/Validator/Rules/OneOfInputObjectsRule.php new file mode 100644 index 000000000..2d59722a2 --- /dev/null +++ b/src/Validator/Rules/OneOfInputObjectsRule.php @@ -0,0 +1,65 @@ + static function (ObjectValueNode $node) use ($context): void { + $type = $context->getInputType(); + + if ($type === null) { + return; + } + + $namedType = Type::getNamedType($type); + if (! ($namedType instanceof InputObjectType) || ! $namedType->isOneOf()) { + return; + } + + $providedFields = []; + foreach ($node->fields as $fieldNode) { + $fieldName = $fieldNode->name->value; + $providedFields[] = $fieldName; + } + + $fieldCount = count($providedFields); + + if ($fieldCount === 0) { + $context->reportError(new Error( + static::oneOfInputObjectExpectedExactlyOneFieldMessage($namedType->name), + [$node] + )); + } elseif ($fieldCount > 1) { + $context->reportError(new Error( + static::oneOfInputObjectExpectedExactlyOneFieldMessage($namedType->name, $fieldCount), + [$node] + )); + } + }, + ]; + } + + public static function oneOfInputObjectExpectedExactlyOneFieldMessage(string $typeName, ?int $providedCount = null): string + { + if ($providedCount === null) { + return "OneOf input object '{$typeName}' must specify exactly one field."; + } + + return "OneOf input object '{$typeName}' must specify exactly one field, but {$providedCount} fields were provided."; + } +} From 3cc0c71e2d18d65c91a9b8ddd7f7dddae33cffa3 Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Thu, 3 Jul 2025 14:04:25 -0600 Subject: [PATCH 03/10] - update validation rule to keep updated with suggestions in https://github.com/graphql/graphql-spec/pull/825 --- src/Validator/Rules/OneOfInputObjectsRule.php | 29 ++++++++++++++++++- tests/Type/OneOfInputObjectTest.php | 7 +++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/Validator/Rules/OneOfInputObjectsRule.php b/src/Validator/Rules/OneOfInputObjectsRule.php index 2d59722a2..8b7a9f429 100644 --- a/src/Validator/Rules/OneOfInputObjectsRule.php +++ b/src/Validator/Rules/OneOfInputObjectsRule.php @@ -32,9 +32,16 @@ public function getVisitor(QueryValidationContext $context): array } $providedFields = []; + $nullFields = []; + foreach ($node->fields as $fieldNode) { $fieldName = $fieldNode->name->value; $providedFields[] = $fieldName; + + // Check if the field value is explicitly null + if ($fieldNode->value->kind === NodeKind::NULL) { + $nullFields[] = $fieldName; + } } $fieldCount = count($providedFields); @@ -44,11 +51,26 @@ public function getVisitor(QueryValidationContext $context): array static::oneOfInputObjectExpectedExactlyOneFieldMessage($namedType->name), [$node] )); - } elseif ($fieldCount > 1) { + + return; + } + + if ($fieldCount > 1) { $context->reportError(new Error( static::oneOfInputObjectExpectedExactlyOneFieldMessage($namedType->name, $fieldCount), [$node] )); + + return; + } + + // At this point, $fieldCount === 1 + if (count($nullFields) > 0) { + // Exactly one field provided, but it's null + $context->reportError(new Error( + static::oneOfInputObjectFieldValueMustNotBeNullMessage($namedType->name, $nullFields[0]), + [$node] + )); } }, ]; @@ -62,4 +84,9 @@ public static function oneOfInputObjectExpectedExactlyOneFieldMessage(string $ty return "OneOf input object '{$typeName}' must specify exactly one field, but {$providedCount} fields were provided."; } + + public static function oneOfInputObjectFieldValueMustNotBeNullMessage(string $typeName, string $fieldName): string + { + return "OneOf input object '{$typeName}' field '{$fieldName}' must be non-null."; + } } diff --git a/tests/Type/OneOfInputObjectTest.php b/tests/Type/OneOfInputObjectTest.php index e80bffac7..d01ec3b9f 100644 --- a/tests/Type/OneOfInputObjectTest.php +++ b/tests/Type/OneOfInputObjectTest.php @@ -124,6 +124,13 @@ public function testOneOfInputObjectSchemaValidation(): void self::assertNotEmpty($result->errors ?? []); self::assertCount(1, $result->errors); self::assertStringContainsString('must specify exactly one field', $result->errors[0]->getMessage()); + + // Invalid query with null field value + $nullQuery = '{ test(input: { stringField: null }) }'; + $result = GraphQL::executeQuery($schema, $nullQuery); + self::assertNotEmpty($result->errors ?? []); + self::assertCount(1, $result->errors); + self::assertStringContainsString('must be non-null', $result->errors[0]->getMessage()); } public function testOneOfIntrospection(): void From e076b3805eac5927685ecd6e2d2e80ed173a06e9 Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Thu, 3 Jul 2025 14:05:24 -0600 Subject: [PATCH 04/10] - update to add pre-coercion of OneOf errors, following this pending PR to graphql-js: https://github.com/graphql/graphql-js/pull/4195 --- src/Utils/Value.php | 36 +++++++++++++++++++++++++++-- tests/Type/OneOfInputObjectTest.php | 36 +++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/Utils/Value.php b/src/Utils/Value.php index 6dd4713ad..d380e2a6f 100644 --- a/src/Utils/Value.php +++ b/src/Utils/Value.php @@ -88,7 +88,7 @@ public static function coerceInputValue($value, InputType $type, ?array $path = $coercedItem = self::coerceInputValue( $itemValue, $itemType, - [...$path ?? [], $index] + $path === null ? [$index] : [...$path, $index] ); if (isset($coercedItem['errors'])) { @@ -132,7 +132,7 @@ public static function coerceInputValue($value, InputType $type, ?array $path = $coercedField = self::coerceInputValue( $fieldValue, $field->getType(), - [...$path ?? [], $fieldName], + $path === null ? [$fieldName] : [...$path, $fieldName], ); if (isset($coercedField['errors'])) { @@ -171,6 +171,38 @@ public static function coerceInputValue($value, InputType $type, ?array $path = ); } + // Validate OneOf constraints if this is a OneOf input type + if ($type->isOneOf()) { + $providedFieldCount = 0; + $nullFieldName = null; + + foreach ($coercedValue as $fieldName => $fieldValue) { + if ($fieldValue !== null) { + $providedFieldCount++; + } else { + $nullFieldName = $fieldName; + } + } + + // Check for null field values first (takes precedence) + if ($nullFieldName !== null) { + $errors = self::add( + $errors, + CoercionError::make("OneOf input object \"{$type->name}\" field \"{$nullFieldName}\" must be non-null.", $path, $value) + ); + } elseif ($providedFieldCount === 0) { + $errors = self::add( + $errors, + CoercionError::make("OneOf input object \"{$type->name}\" must specify exactly one field.", $path, $value) + ); + } elseif ($providedFieldCount > 1) { + $errors = self::add( + $errors, + CoercionError::make("OneOf input object \"{$type->name}\" must specify exactly one field.", $path, $value) + ); + } + } + return $errors === [] ? self::ofValue($type->parseValue($coercedValue)) : self::ofErrors($errors); diff --git a/tests/Type/OneOfInputObjectTest.php b/tests/Type/OneOfInputObjectTest.php index d01ec3b9f..12d41d6b2 100644 --- a/tests/Type/OneOfInputObjectTest.php +++ b/tests/Type/OneOfInputObjectTest.php @@ -8,6 +8,7 @@ use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; use GraphQL\Type\Schema; +use GraphQL\Utils\Value; use PHPUnit\Framework\TestCase; class OneOfInputObjectTest extends TestCase @@ -198,4 +199,39 @@ public function testOneOfIntrospection(): void self::assertTrue($oneOfType['isOneOf']); self::assertFalse($regularType['isOneOf']); // Should be false for regular input objects } + + public function testOneOfCoercionValidation(): void + { + $oneOfType = new InputObjectType([ + 'name' => 'OneOfInput', + 'fields' => [ + 'stringField' => Type::string(), + 'intField' => Type::int(), + ], + 'isOneOf' => true, + ]); + + // Test valid input (exactly one field) + $validResult = Value::coerceInputValue(['stringField' => 'test'], $oneOfType); + $this->assertNull($validResult['errors']); + $this->assertEquals(['stringField' => 'test'], $validResult['value']); + + // Test invalid input (no fields) + $noFieldsResult = Value::coerceInputValue([], $oneOfType); + $this->assertNotNull($noFieldsResult['errors']); + $this->assertCount(1, $noFieldsResult['errors']); + $this->assertEquals('OneOf input object "OneOfInput" must specify exactly one field.', $noFieldsResult['errors'][0]->getMessage()); + + // Test invalid input (multiple fields) + $multipleFieldsResult = Value::coerceInputValue(['stringField' => 'test', 'intField' => 42], $oneOfType); + $this->assertNotNull($multipleFieldsResult['errors']); + $this->assertCount(1, $multipleFieldsResult['errors']); + $this->assertEquals('OneOf input object "OneOfInput" must specify exactly one field.', $multipleFieldsResult['errors'][0]->getMessage()); + + // Test invalid input (null field value) + $nullFieldResult = Value::coerceInputValue(['stringField' => null], $oneOfType); + $this->assertNotNull($nullFieldResult['errors']); + $this->assertCount(1, $nullFieldResult['errors']); + $this->assertEquals('OneOf input object "OneOfInput" field "stringField" must be non-null.', $nullFieldResult['errors'][0]->getMessage()); + } } From 3b874adce0ed66e9286b80caa91602600c9a59da Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Thu, 3 Jul 2025 14:06:01 -0600 Subject: [PATCH 05/10] - add unreleased changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7afd0b453..013c7939e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Added + +- Add support for `@oneOf` input object directive - enables "input unions" where exactly one field must be provided + ## v15.20.0 ### Added From b64fdd78e7449e92c4a66af51214a8fad9a6df95 Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Thu, 3 Jul 2025 14:08:44 -0600 Subject: [PATCH 06/10] - phpstan corrections --- src/Utils/Value.php | 6 +++--- tests/Type/OneOfInputObjectTest.php | 22 +++++++++++----------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Utils/Value.php b/src/Utils/Value.php index d380e2a6f..390f2a726 100644 --- a/src/Utils/Value.php +++ b/src/Utils/Value.php @@ -175,15 +175,15 @@ public static function coerceInputValue($value, InputType $type, ?array $path = if ($type->isOneOf()) { $providedFieldCount = 0; $nullFieldName = null; - + foreach ($coercedValue as $fieldName => $fieldValue) { if ($fieldValue !== null) { - $providedFieldCount++; + ++$providedFieldCount; } else { $nullFieldName = $fieldName; } } - + // Check for null field values first (takes precedence) if ($nullFieldName !== null) { $errors = self::add( diff --git a/tests/Type/OneOfInputObjectTest.php b/tests/Type/OneOfInputObjectTest.php index 12d41d6b2..19dc30b70 100644 --- a/tests/Type/OneOfInputObjectTest.php +++ b/tests/Type/OneOfInputObjectTest.php @@ -213,25 +213,25 @@ public function testOneOfCoercionValidation(): void // Test valid input (exactly one field) $validResult = Value::coerceInputValue(['stringField' => 'test'], $oneOfType); - $this->assertNull($validResult['errors']); - $this->assertEquals(['stringField' => 'test'], $validResult['value']); + self::assertNull($validResult['errors']); + self::assertEquals(['stringField' => 'test'], $validResult['value']); // Test invalid input (no fields) $noFieldsResult = Value::coerceInputValue([], $oneOfType); - $this->assertNotNull($noFieldsResult['errors']); - $this->assertCount(1, $noFieldsResult['errors']); - $this->assertEquals('OneOf input object "OneOfInput" must specify exactly one field.', $noFieldsResult['errors'][0]->getMessage()); + self::assertNotNull($noFieldsResult['errors']); + self::assertCount(1, $noFieldsResult['errors']); + self::assertEquals('OneOf input object "OneOfInput" must specify exactly one field.', $noFieldsResult['errors'][0]->getMessage()); // Test invalid input (multiple fields) $multipleFieldsResult = Value::coerceInputValue(['stringField' => 'test', 'intField' => 42], $oneOfType); - $this->assertNotNull($multipleFieldsResult['errors']); - $this->assertCount(1, $multipleFieldsResult['errors']); - $this->assertEquals('OneOf input object "OneOfInput" must specify exactly one field.', $multipleFieldsResult['errors'][0]->getMessage()); + self::assertNotNull($multipleFieldsResult['errors']); + self::assertCount(1, $multipleFieldsResult['errors']); + self::assertEquals('OneOf input object "OneOfInput" must specify exactly one field.', $multipleFieldsResult['errors'][0]->getMessage()); // Test invalid input (null field value) $nullFieldResult = Value::coerceInputValue(['stringField' => null], $oneOfType); - $this->assertNotNull($nullFieldResult['errors']); - $this->assertCount(1, $nullFieldResult['errors']); - $this->assertEquals('OneOf input object "OneOfInput" field "stringField" must be non-null.', $nullFieldResult['errors'][0]->getMessage()); + self::assertNotNull($nullFieldResult['errors']); + self::assertCount(1, $nullFieldResult['errors']); + self::assertEquals('OneOf input object "OneOfInput" field "stringField" must be non-null.', $nullFieldResult['errors'][0]->getMessage()); } } From b7a37e15bc3ad67dd1020aabc6c74ec332a5965b Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Fri, 4 Jul 2025 09:05:08 -0600 Subject: [PATCH 07/10] Update CHANGELOG.md Co-authored-by: Benedikt Franke --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 013c7939e..f96ef3a3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ You can find and compare releases at the [GitHub release page](https://github.co ### Added -- Add support for `@oneOf` input object directive - enables "input unions" where exactly one field must be provided +- Add support for `@oneOf` input object directive - enables "input unions" where exactly one field must be provided https://github.com/webonyx/graphql-php/pull/1715 ## v15.20.0 From 64be6d241d6ba6135b9918983d5ce10e960434fe Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Fri, 4 Jul 2025 10:33:52 -0600 Subject: [PATCH 08/10] - add test case ensuring that InputObjectType has at least one or more fields - removed test and corresponding InvariantViolation that oneOf input types must not have deprecated fields. This was a mistake. I think in my mind I was thinking that if only 1 field was defined it should not be deprecated, but this doesn't match that intent and that's not part of the spec from what I can tell anyway so I've removed it --- src/Type/Definition/InputObjectType.php | 17 ++++++----------- tests/Type/OneOfInputObjectTest.php | 22 ++++++++++++++++++---- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/Type/Definition/InputObjectType.php b/src/Type/Definition/InputObjectType.php index d0358fe17..fcfbedcae 100644 --- a/src/Type/Definition/InputObjectType.php +++ b/src/Type/Definition/InputObjectType.php @@ -69,12 +69,6 @@ public function __construct(array $config) $this->config = $config; } - /** Returns true if this is a oneOf input object type. */ - public function isOneOf(): bool - { - return $this->isOneOf; - } - /** @throws InvariantViolation */ public function getField(string $name): InputObjectField { @@ -107,6 +101,12 @@ public function hasField(string $name): bool return isset($this->fields[$name]); } + /** Returns true if this is a oneOf input object type. */ + public function isOneOf(): bool + { + return $this->isOneOf; + } + /** * @throws InvariantViolation * @@ -244,11 +244,6 @@ private function validateOneOfConstraints(array $fields): void if ($field->defaultValueExists()) { throw new InvariantViolation("OneOf input object type {$this->name} field {$fieldName} cannot have a default value."); } - - // OneOf fields cannot be deprecated (optional constraint for now) - if ($field->isDeprecated()) { - throw new InvariantViolation("OneOf input object type {$this->name} field {$fieldName} cannot be deprecated."); - } } } diff --git a/tests/Type/OneOfInputObjectTest.php b/tests/Type/OneOfInputObjectTest.php index 395ae4d21..2110bedbc 100644 --- a/tests/Type/OneOfInputObjectTest.php +++ b/tests/Type/OneOfInputObjectTest.php @@ -81,6 +81,20 @@ public function testOneOfInputObjectRejectsDefaultValues(): void $oneOfInput->assertValid(); } + public function testOneOfInputObjectRequiresAtLeastOneField(): void + { + $oneOfInput = new InputObjectType([ + 'name' => 'OneOfInput', + 'isOneOf' => true, + 'fields' => [], // Empty fields array should fail + ]); + + $this->expectException(InvariantViolation::class); + $this->expectExceptionMessage('OneOf input object type OneOfInput must define one or more fields'); + + $oneOfInput->assertValid(); + } + public function testOneOfInputObjectSchemaValidation(): void { $oneOfInput = new InputObjectType([ @@ -110,26 +124,26 @@ public function testOneOfInputObjectSchemaValidation(): void // Valid query with exactly one field $validQuery = '{ test(input: { stringField: "hello" }) }'; $result = GraphQL::executeQuery($schema, $validQuery); - self::assertEmpty($result->errors ?? []); + self::assertEmpty($result->errors); // Invalid query with multiple fields $invalidQuery = '{ test(input: { stringField: "hello", intField: 42 }) }'; $result = GraphQL::executeQuery($schema, $invalidQuery); - self::assertNotEmpty($result->errors ?? []); + self::assertNotEmpty($result->errors); self::assertCount(1, $result->errors); self::assertStringContainsString('must specify exactly one field', $result->errors[0]->getMessage()); // Invalid query with no fields $emptyQuery = '{ test(input: {}) }'; $result = GraphQL::executeQuery($schema, $emptyQuery); - self::assertNotEmpty($result->errors ?? []); + self::assertNotEmpty($result->errors); self::assertCount(1, $result->errors); self::assertStringContainsString('must specify exactly one field', $result->errors[0]->getMessage()); // Invalid query with null field value $nullQuery = '{ test(input: { stringField: null }) }'; $result = GraphQL::executeQuery($schema, $nullQuery); - self::assertNotEmpty($result->errors ?? []); + self::assertNotEmpty($result->errors); self::assertCount(1, $result->errors); self::assertStringContainsString('must be non-null', $result->errors[0]->getMessage()); } From 646fef1c585da0d6f0126619651f571b1847f657 Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Fri, 4 Jul 2025 10:38:34 -0600 Subject: [PATCH 09/10] - revert change to path in coerceInputValue --- src/Utils/Value.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils/Value.php b/src/Utils/Value.php index b05ce9ebc..d2cdcccf1 100644 --- a/src/Utils/Value.php +++ b/src/Utils/Value.php @@ -132,7 +132,7 @@ public static function coerceInputValue($value, InputType $type, ?array $path = $coercedField = self::coerceInputValue( $fieldValue, $field->getType(), - $path === null ? [$fieldName] : [...$path, $fieldName], + [...$path ?? [], $fieldName], ); if (isset($coercedField['errors'])) { From 82bd7cf696e65352ca3913fcfadbe0e5926c9216 Mon Sep 17 00:00:00 2001 From: Jason Bahl Date: Mon, 7 Jul 2025 09:43:58 -0600 Subject: [PATCH 10/10] - move isOneOf below description, above fields --- src/Type/Definition/InputObjectType.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type/Definition/InputObjectType.php b/src/Type/Definition/InputObjectType.php index fcfbedcae..6f1e374c2 100644 --- a/src/Type/Definition/InputObjectType.php +++ b/src/Type/Definition/InputObjectType.php @@ -18,8 +18,8 @@ * @phpstan-type InputObjectConfig array{ * name?: string|null, * description?: string|null, - * fields: iterable|callable(): iterable, * isOneOf?: bool|null, + * fields: iterable|callable(): iterable, * parseValue?: ParseValueFn|null, * astNode?: InputObjectTypeDefinitionNode|null, * extensionASTNodes?: array|null