From 43da702c5d513929c3a051ed6ed16b96637f7924 Mon Sep 17 00:00:00 2001 From: murtukov Date: Sun, 2 Nov 2025 06:57:16 +0100 Subject: [PATCH 1/7] Refactor validation mappings and tests * Adjust validation for nested and partial input objects. * Refactor metadata handling in `InputValidator`. * Add test for `partialInputObjectsCollectionValidation`. * Remove test `testOnlyPassedFieldsValidated`. * Update validation constraints in YAML configuration files. --- src/Validator/InputValidator.php | 38 ++++++++--- .../validator/mapping/Address.types.yml | 5 +- .../validator/mapping/Country.types.yml | 13 ++++ .../validator/mapping/Mutation.types.yml | 8 +-- .../config/validator/mapping/Period.types.yml | 4 +- .../config/validator/mapping/Person.types.yml | 14 ---- tests/Functional/Validator/ExpectedErrors.php | 28 ++++++++ .../Validator/InputValidatorTest.php | 64 ++++++++++++++----- 8 files changed, 127 insertions(+), 47 deletions(-) create mode 100644 tests/Functional/App/config/validator/mapping/Country.types.yml delete mode 100644 tests/Functional/App/config/validator/mapping/Person.types.yml diff --git a/src/Validator/InputValidator.php b/src/Validator/InputValidator.php index 8059456ed..27a82e9c1 100644 --- a/src/Validator/InputValidator.php +++ b/src/Validator/InputValidator.php @@ -124,13 +124,27 @@ private function createValidator(MetadataFactory $metadataFactory): ValidatorInt return $builder->getValidator(); } + private function getMetadata(ValidationNode $rootObject): ObjectMetadata + { + // Return existing metadata if present + if ($this->metadataFactory->hasMetadataFor($rootObject)) { + return $this->metadataFactory->getMetadataFor($rootObject); + } + + // Create new metadata and add it to the factory + $metadata = new ObjectMetadata($rootObject); + $this->metadataFactory->addMetadata($metadata); + + return $metadata; + } + /** * Creates a composition of ValidationNode objects from args * and simultaneously applies to them validation constraints. */ private function buildValidationTree(ValidationNode $rootObject, iterable $fields, array $classValidation, array $inputData): ValidationNode { - $metadata = new ObjectMetadata($rootObject); + $metadata = $this->getMetadata($rootObject); if (!empty($classValidation)) { $this->applyClassValidation($metadata, $classValidation); @@ -140,13 +154,7 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field $property = $arg['name'] ?? $name; $config = static::normalizeConfig($arg['validation'] ?? []); - if (!array_key_exists($property, $inputData)) { - // This field was not provided in the inputData. Do not attempt to validate it. - continue; - } - if (isset($config['cascade']) && isset($inputData[$property])) { - $groups = $config['cascade']; $argType = $this->unclosure($arg['type']); /** @var ObjectType|InputObjectType $type */ @@ -159,18 +167,29 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field } $valid = new Valid(); + $groups = $config['cascade']; if (!empty($groups)) { $valid->groups = $groups; } + // Apply the Assert/Valid constraint for a recursive validation. + // For more details see https://symfony.com/doc/current/reference/constraints/Valid.html $metadata->addPropertyConstraint($property, $valid); + + // Skip the rest as the validation was delegated to the nested object. + continue; } else { $rootObject->$property = $inputData[$property] ?? null; } + if ($metadata->hasPropertyMetadata($property)) { + continue; + } + $config = static::normalizeConfig($config); + // Apply validation constraints for the property foreach ($config as $key => $value) { switch ($key) { case 'link': @@ -200,17 +219,16 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field } break; - case 'constraints': + case 'constraints': // Add constraint from the yml config $metadata->addPropertyConstraints($property, $value); break; case 'cascade': + // Cascade validation was already handled recursively. break; } } } - $this->metadataFactory->addMetadata($metadata); - return $rootObject; } diff --git a/tests/Functional/App/config/validator/mapping/Address.types.yml b/tests/Functional/App/config/validator/mapping/Address.types.yml index 3426bd408..1e3a6c343 100644 --- a/tests/Functional/App/config/validator/mapping/Address.types.yml +++ b/tests/Functional/App/config/validator/mapping/Address.types.yml @@ -14,10 +14,13 @@ Address: - Choice: groups: ['group1'] choices: ['New York', 'Berlin', 'Tokyo'] + country: + type: Country + validation: cascade zipCode: type: Int! validation: - Expression: "service_validator.isZipCodeValid(value)" period: - type: Period! + type: Period validation: cascade diff --git a/tests/Functional/App/config/validator/mapping/Country.types.yml b/tests/Functional/App/config/validator/mapping/Country.types.yml new file mode 100644 index 000000000..09f2f266e --- /dev/null +++ b/tests/Functional/App/config/validator/mapping/Country.types.yml @@ -0,0 +1,13 @@ +Country: + type: input-object + config: + fields: + name: + type: String + validation: + - NotBlank: + allowNull: true + officialLanguage: + type: String + validation: + - Choice: ['en', 'de', 'fr'] diff --git a/tests/Functional/App/config/validator/mapping/Mutation.types.yml b/tests/Functional/App/config/validator/mapping/Mutation.types.yml index e320f09ac..6c85fd67c 100644 --- a/tests/Functional/App/config/validator/mapping/Mutation.types.yml +++ b/tests/Functional/App/config/validator/mapping/Mutation.types.yml @@ -139,10 +139,10 @@ Mutation: cascade: groups: ['group2'] - onlyPassedFieldsValidation: + partialInputObjectsCollectionValidation: type: Boolean - resolve: '@=m("mutation_mock", args)' + resolve: "@=m('mutation_mock', args)" args: - person: + addresses: + type: '[Address]' validation: cascade - type: Person! diff --git a/tests/Functional/App/config/validator/mapping/Period.types.yml b/tests/Functional/App/config/validator/mapping/Period.types.yml index 909315b75..557506553 100644 --- a/tests/Functional/App/config/validator/mapping/Period.types.yml +++ b/tests/Functional/App/config/validator/mapping/Period.types.yml @@ -3,7 +3,7 @@ Period: config: fields: startDate: - type: String! + type: String validation: - Date: ~ - Overblog\GraphQLBundle\Tests\Functional\App\Validator\AtLeastOneOf: @@ -12,7 +12,7 @@ Period: message: "Year should be GreaterThanOrEqual -100." includeInternalMessages: false endDate: - type: String! + type: String validation: - Expression: "this.getParent().getName() === 'Address'" - Date: ~ diff --git a/tests/Functional/App/config/validator/mapping/Person.types.yml b/tests/Functional/App/config/validator/mapping/Person.types.yml deleted file mode 100644 index bb8e29642..000000000 --- a/tests/Functional/App/config/validator/mapping/Person.types.yml +++ /dev/null @@ -1,14 +0,0 @@ -Person: - type: input-object - config: - fields: - firstName: - type: String - validation: - - NotBlank: ~ - - NotNull: ~ - surname: - type: String - validation: - - NotBlank: ~ - - NotNull: ~ diff --git a/tests/Functional/Validator/ExpectedErrors.php b/tests/Functional/Validator/ExpectedErrors.php index e4f5b34df..7aaa53759 100644 --- a/tests/Functional/Validator/ExpectedErrors.php +++ b/tests/Functional/Validator/ExpectedErrors.php @@ -79,6 +79,34 @@ final class ExpectedErrors ], ]; + public const PARTIAL_INPUT_OBJECTS_COLLECTION = [ + 'message' => 'validation', + 'locations' => [['line' => 3, 'column' => 17]], + 'path' => ['partialInputObjectsCollectionValidation'], + 'extensions' => [ + 'validation' => [ + 'addresses[0].country.officialLanguage' => [ + 0 => [ + 'message' => 'The value you selected is not a valid choice.', + 'code' => '8e179f1b-97aa-4560-a02f-2a8b42e49df7' + ], + ], + 'addresses[1].country.name' => [ + 0 => [ + 'message' => 'This value should not be blank.', + 'code' => 'c1051bb4-d103-4f74-8988-acbcafc7fdc3' + ], + ], + 'addresses[1].period.endDate' => [ + 0 => [ + 'message' => 'This value should be greater than "2000-01-01".', + 'code' => '778b7ae0-84d3-481a-9dec-35fdb64b1d78' + ] + ] + ] + ] + ]; + public static function simpleValidation(string $fieldName): array { return [ diff --git a/tests/Functional/Validator/InputValidatorTest.php b/tests/Functional/Validator/InputValidatorTest.php index f454ab057..7deae5eed 100644 --- a/tests/Functional/Validator/InputValidatorTest.php +++ b/tests/Functional/Validator/InputValidatorTest.php @@ -87,22 +87,6 @@ public function testLinkedConstraintsValidationPasses(): void $this->assertTrue($result['data']['linkedConstraintsValidation']); } - public function testOnlyPassedFieldsValidated(): void - { - $query = ' - mutation { - onlyPassedFieldsValidation( - person: { firstName: "Joe" } - ) - } - '; - - $result = $this->executeGraphQLRequest($query); - - $this->assertTrue(empty($result['errors'])); - $this->assertTrue($result['data']['onlyPassedFieldsValidation']); - } - public function testLinkedConstraintsValidationFails(): void { $query = ' @@ -393,4 +377,52 @@ public function testAutoValidationAutoThrowWithGroupsFails(): void $this->assertSame(ExpectedErrors::cascadeWithGroups('autoValidationAutoThrowWithGroups'), $result['errors'][0]); $this->assertNull($result['data']['autoValidationAutoThrowWithGroups']); } + + public function testPartialInputObjectsCollectionValidation(): void + { + $query = ' + mutation { + partialInputObjectsCollectionValidation( + addresses: [ + { + street: "Washington Street" + city: "Berlin" + zipCode: 10000 + # Country is present, but the language is invalid + country: { + name: "Germany" + officialLanguage: "ru" + } + # Period is completely missing, skip validation + }, + { + street: "Washington Street" + city: "New York" + zipCode: 10000 + # Country is partially present + country: { + name: "" # Name should not be blank + # language is missing + } + period: { + startDate: "2000-01-01" + endDate: "1990-01-01" + } + }, + { + street: "Washington Street" + city: "New York" + zipCode: 10000 + country: {} # Empty input object, skip validation + period: {} # Empty input object, skip validation + } + ] + ) + } + '; + + $result = $this->executeGraphQLRequest($query); + $this->assertSame(ExpectedErrors::PARTIAL_INPUT_OBJECTS_COLLECTION, $result['errors'][0]); + $this->assertNull($result['data']['partialInputObjectsCollectionValidation']); + } } From eecaaeec4621c67f50026ec01359b5c89fbf3e08 Mon Sep 17 00:00:00 2001 From: murtukov Date: Sun, 2 Nov 2025 19:38:53 +0100 Subject: [PATCH 2/7] Fix formatting inconsistencies in `ExpectedErrors` test definitions * Add missing trailing commas to array elements. --- tests/Functional/Validator/ExpectedErrors.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/Functional/Validator/ExpectedErrors.php b/tests/Functional/Validator/ExpectedErrors.php index 7aaa53759..5aef2fb9c 100644 --- a/tests/Functional/Validator/ExpectedErrors.php +++ b/tests/Functional/Validator/ExpectedErrors.php @@ -88,23 +88,23 @@ final class ExpectedErrors 'addresses[0].country.officialLanguage' => [ 0 => [ 'message' => 'The value you selected is not a valid choice.', - 'code' => '8e179f1b-97aa-4560-a02f-2a8b42e49df7' + 'code' => '8e179f1b-97aa-4560-a02f-2a8b42e49df7', ], ], 'addresses[1].country.name' => [ 0 => [ 'message' => 'This value should not be blank.', - 'code' => 'c1051bb4-d103-4f74-8988-acbcafc7fdc3' + 'code' => 'c1051bb4-d103-4f74-8988-acbcafc7fdc3', ], ], 'addresses[1].period.endDate' => [ 0 => [ 'message' => 'This value should be greater than "2000-01-01".', - 'code' => '778b7ae0-84d3-481a-9dec-35fdb64b1d78' - ] - ] - ] - ] + 'code' => '778b7ae0-84d3-481a-9dec-35fdb64b1d78', + ], + ], + ], + ], ]; public static function simpleValidation(string $fieldName): array From 703cda6837ebb1fb57e8ea7428d00e440730e752 Mon Sep 17 00:00:00 2001 From: murtukov Date: Mon, 3 Nov 2025 00:19:54 +0100 Subject: [PATCH 3/7] Reduce cyclomatic complexity --- src/Validator/InputValidator.php | 147 ++++++++++++++++++------------- 1 file changed, 85 insertions(+), 62 deletions(-) diff --git a/src/Validator/InputValidator.php b/src/Validator/InputValidator.php index 27a82e9c1..acd85d2bb 100644 --- a/src/Validator/InputValidator.php +++ b/src/Validator/InputValidator.php @@ -151,85 +151,108 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field } foreach ($fields as $name => $arg) { - $property = $arg['name'] ?? $name; + $property = $this->resolvePropertyName($name, $arg); $config = static::normalizeConfig($arg['validation'] ?? []); - if (isset($config['cascade']) && isset($inputData[$property])) { - $argType = $this->unclosure($arg['type']); + if ($this->shouldCascade($config, $inputData, $property)) { + $this->handleCascade($rootObject, $property, $arg, $config, $inputData[$property]); + continue; // delegated to nested object + } - /** @var ObjectType|InputObjectType $type */ - $type = Type::getNamedType($argType); + // assign scalar/null value when not cascading + $rootObject->$property = $inputData[$property] ?? null; - if (static::isListOfType($argType)) { - $rootObject->$property = $this->createCollectionNode($inputData[$property], $type, $rootObject); - } else { - $rootObject->$property = $this->createObjectNode($inputData[$property], $type, $rootObject); - } + if ($metadata->hasPropertyMetadata($property)) { + continue; + } + + $config = static::normalizeConfig($config); + $this->applyConfigToMetadata($metadata, $property, $config); + } - $valid = new Valid(); - $groups = $config['cascade']; + return $rootObject; + } - if (!empty($groups)) { - $valid->groups = $groups; - } + private function resolvePropertyName(string|int $name, array $arg): string|int + { + return $arg['name'] ?? $name; + } - // Apply the Assert/Valid constraint for a recursive validation. - // For more details see https://symfony.com/doc/current/reference/constraints/Valid.html - $metadata->addPropertyConstraint($property, $valid); + private function shouldCascade(array $config, array $inputData, string|int $property): bool + { + return isset($config['cascade']) && isset($inputData[$property]); + } - // Skip the rest as the validation was delegated to the nested object. - continue; - } else { - $rootObject->$property = $inputData[$property] ?? null; - } + private function handleCascade(ValidationNode $rootObject, string|int $property, array $arg, array $config, mixed $value): void + { + $argType = $this->unclosure($arg['type']); + /** @var ObjectType|InputObjectType $type */ + $type = Type::getNamedType($argType); - if ($metadata->hasPropertyMetadata($property)) { - continue; - } + if (static::isListOfType($argType)) { + $rootObject->$property = $this->createCollectionNode($value, $type, $rootObject); + } else { + $rootObject->$property = $this->createObjectNode($value, $type, $rootObject); + } - $config = static::normalizeConfig($config); + $this->addValidConstraint($this->getMetadata($rootObject), (string) $property, $config['cascade']); + } - // Apply validation constraints for the property - foreach ($config as $key => $value) { - switch ($key) { - case 'link': - [$fqcn, $classProperty, $type] = $value; + private function addValidConstraint(ObjectMetadata $metadata, string $property, array $groups): void + { + $valid = new Valid(); + if (!empty($groups)) { + $valid->groups = $groups; + } + // Apply the Assert/Valid constraint for a recursive validation. + // For more details see https://symfony.com/doc/current/reference/constraints/Valid.html + $metadata->addPropertyConstraint($property, $valid); + } - if (!in_array($fqcn, $this->cachedMetadata)) { - /** @phpstan-ignore-next-line */ - $this->cachedMetadata[$fqcn] = $this->defaultValidator->getMetadataFor($fqcn); - } + private function applyConfigToMetadata(ObjectMetadata $metadata, string|int $property, array $config): void + { + foreach ($config as $key => $value) { + switch ($key) { + case 'link': + $this->applyLinkConstraints($metadata, (string) $property, $value); + break; + case 'constraints': + // Add constraints from the yml config + $metadata->addPropertyConstraints((string) $property, $value); + break; + case 'cascade': + // Cascade validation was already handled recursively. + break; + } + } + } - // Get metadata from the property and it's getters - $propertyMetadata = $this->cachedMetadata[$fqcn]->getPropertyMetadata($classProperty); - - foreach ($propertyMetadata as $memberMetadata) { - // Allow only constraints specified by the "link" matcher - if (self::TYPE_GETTER === $type) { - if (!$memberMetadata instanceof GetterMetadata) { - continue; - } - } elseif (self::TYPE_PROPERTY === $type) { - if (!$memberMetadata instanceof PropertyMetadata) { - continue; - } - } - - $metadata->addPropertyConstraints($property, $memberMetadata->getConstraints()); - } + private function applyLinkConstraints(ObjectMetadata $metadata, string $property, array $link): void + { + [$fqcn, $classProperty, $type] = $link; + + if (!in_array($fqcn, $this->cachedMetadata)) { + /** @phpstan-ignore-next-line */ + $this->cachedMetadata[$fqcn] = $this->defaultValidator->getMetadataFor($fqcn); + } + + // Get metadata from the property and its getters + $propertyMetadata = $this->cachedMetadata[$fqcn]->getPropertyMetadata($classProperty); - break; - case 'constraints': // Add constraint from the yml config - $metadata->addPropertyConstraints($property, $value); - break; - case 'cascade': - // Cascade validation was already handled recursively. - break; + foreach ($propertyMetadata as $memberMetadata) { + // Allow only constraints specified by the "link" matcher + if (self::TYPE_GETTER === $type) { + if (!$memberMetadata instanceof GetterMetadata) { + continue; + } + } elseif (self::TYPE_PROPERTY === $type) { + if (!$memberMetadata instanceof PropertyMetadata) { + continue; } } - } - return $rootObject; + $metadata->addPropertyConstraints($property, $memberMetadata->getConstraints()); + } } private static function isListOfType(GeneratedTypeInterface|ListOfType|NonNull $type): bool From 806fc9dd672d2a2211a33801b204d5b686d33396 Mon Sep 17 00:00:00 2001 From: murtukov Date: Mon, 3 Nov 2025 07:10:22 +0100 Subject: [PATCH 4/7] Refactor InputValidator * Extract utility methods to `Utils` class. * Simplify `InputValidator` by delegating common logic to `Utils`. * Improve readability and reduce duplication across validation methods. * Enhance docblocks for better clarity. --- src/Validator/InputValidator.php | 137 ++++++++++--------------------- src/Validator/Utils.php | 86 +++++++++++++++++++ 2 files changed, 128 insertions(+), 95 deletions(-) create mode 100644 src/Validator/Utils.php diff --git a/src/Validator/InputValidator.php b/src/Validator/InputValidator.php index acd85d2bb..a5ae2714b 100644 --- a/src/Validator/InputValidator.php +++ b/src/Validator/InputValidator.php @@ -4,15 +4,11 @@ namespace Overblog\GraphQLBundle\Validator; -use Closure; use GraphQL\Type\Definition\InputObjectType; -use GraphQL\Type\Definition\ListOfType; -use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Type\Definition\Type; use Overblog\GraphQLBundle\Definition\ResolverArgs; -use Overblog\GraphQLBundle\Definition\Type\GeneratedTypeInterface; use Overblog\GraphQLBundle\Validator\Exception\ArgumentsValidationException; use Overblog\GraphQLBundle\Validator\Mapping\MetadataFactory; use Overblog\GraphQLBundle\Validator\Mapping\ObjectMetadata; @@ -61,6 +57,8 @@ public function __construct( } /** + * Entry point. + * * @throws ArgumentsValidationException */ public function validate(string|array|null $groups = null, bool $throw = true): ?ConstraintViolationListInterface @@ -72,17 +70,14 @@ public function validate(string|array|null $groups = null, bool $throw = true): $this->resolverArgs ); - $classMapping = $this->mergeClassValidation(); - $this->buildValidationTree( $rootNode, $this->info->fieldDefinition->config['args'] ?? [], - $classMapping, + Utils::getClassLevelConstraints($this->info), $this->resolverArgs->args->getArrayCopy() ); $validator = $this->createValidator($this->metadataFactory); - $errors = $validator->validate($rootNode, null, $groups); if ($throw && $errors->count() > 0) { @@ -92,22 +87,10 @@ public function validate(string|array|null $groups = null, bool $throw = true): } } - private function mergeClassValidation(): array - { - /** @phpstan-ignore-next-line */ - $common = static::normalizeConfig($this->info->parentType->config['validation'] ?? []); - /** @phpstan-ignore-next-line */ - $specific = static::normalizeConfig($this->info->fieldDefinition->config['validation'] ?? []); - - return array_filter([ - 'link' => $specific['link'] ?? $common['link'] ?? null, - 'constraints' => [ - ...($common['constraints'] ?? []), - ...($specific['constraints'] ?? []), - ], - ]); - } - + /** + * Creates a validator with a custom metadata factory. The metadata factory + * is used to properly map validation constraints to ValidationNode objects. + */ private function createValidator(MetadataFactory $metadataFactory): ValidatorInterface { $builder = Validation::createValidatorBuilder() @@ -124,6 +107,10 @@ private function createValidator(MetadataFactory $metadataFactory): ValidatorInt return $builder->getValidator(); } + /** + * Either returns an existing metadata object related to + * the ValidationNode object or creates a new one. + */ private function getMetadata(ValidationNode $rootObject): ObjectMetadata { // Return existing metadata if present @@ -139,8 +126,8 @@ private function getMetadata(ValidationNode $rootObject): ObjectMetadata } /** - * Creates a composition of ValidationNode objects from args - * and simultaneously applies to them validation constraints. + * Creates a map of ValidationNode objects from args and + * simultaneously applies validation constraints to them. */ private function buildValidationTree(ValidationNode $rootObject, iterable $fields, array $classValidation, array $inputData): ValidationNode { @@ -151,8 +138,8 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field } foreach ($fields as $name => $arg) { - $property = $this->resolvePropertyName($name, $arg); - $config = static::normalizeConfig($arg['validation'] ?? []); + $property = $arg['name'] ?? $name; + $config = Utils::normalizeConfig($arg['validation'] ?? []); if ($this->shouldCascade($config, $inputData, $property)) { $this->handleCascade($rootObject, $property, $arg, $config, $inputData[$property]); @@ -166,58 +153,65 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field continue; } - $config = static::normalizeConfig($config); - $this->applyConfigToMetadata($metadata, $property, $config); + $this->applyPropertyConstraints($metadata, $property, Utils::normalizeConfig($config)); } return $rootObject; } - private function resolvePropertyName(string|int $name, array $arg): string|int - { - return $arg['name'] ?? $name; - } - private function shouldCascade(array $config, array $inputData, string|int $property): bool { return isset($config['cascade']) && isset($inputData[$property]); } + /** + * Creates a new nested ValidationNode object or a collection of them + * based on the type of the argument and applies the cascade validation. + */ private function handleCascade(ValidationNode $rootObject, string|int $property, array $arg, array $config, mixed $value): void { - $argType = $this->unclosure($arg['type']); + $argType = Utils::unclosure($arg['type']); /** @var ObjectType|InputObjectType $type */ $type = Type::getNamedType($argType); - if (static::isListOfType($argType)) { + if (Utils::isListOfType($argType)) { $rootObject->$property = $this->createCollectionNode($value, $type, $rootObject); } else { $rootObject->$property = $this->createObjectNode($value, $type, $rootObject); } + // Mark the property for recursive validation $this->addValidConstraint($this->getMetadata($rootObject), (string) $property, $config['cascade']); } + /** + * Applies the Assert\Valid constraint to enable a recursive validation. + * + * @link https://symfony.com/doc/current/reference/constraints/Valid.html Docs + */ private function addValidConstraint(ObjectMetadata $metadata, string $property, array $groups): void { $valid = new Valid(); if (!empty($groups)) { $valid->groups = $groups; } - // Apply the Assert/Valid constraint for a recursive validation. - // For more details see https://symfony.com/doc/current/reference/constraints/Valid.html + $metadata->addPropertyConstraint($property, $valid); } - private function applyConfigToMetadata(ObjectMetadata $metadata, string|int $property, array $config): void + /** + * Adds property constraints to the metadata object related to a ValidationNode object. + */ + private function applyPropertyConstraints(ObjectMetadata $metadata, string|int $property, array $config): void { foreach ($config as $key => $value) { switch ($key) { case 'link': - $this->applyLinkConstraints($metadata, (string) $property, $value); + // Add constraints from the linked class + $this->addLinkedConstraints((string) $property, $value, $metadata); break; case 'constraints': - // Add constraints from the yml config + // Add constraints from the yml config directly $metadata->addPropertyConstraints((string) $property, $value); break; case 'cascade': @@ -227,7 +221,7 @@ private function applyConfigToMetadata(ObjectMetadata $metadata, string|int $pro } } - private function applyLinkConstraints(ObjectMetadata $metadata, string $property, array $link): void + private function addLinkedConstraints(string $property, array $link, ObjectMetadata $metadata, ): void { [$fqcn, $classProperty, $type] = $link; @@ -255,14 +249,6 @@ private function applyLinkConstraints(ObjectMetadata $metadata, string $property } } - private static function isListOfType(GeneratedTypeInterface|ListOfType|NonNull $type): bool - { - if ($type instanceof ListOfType || ($type instanceof NonNull && $type->getWrappedType() instanceof ListOfType)) { - return true; - } - - return false; - } private function createCollectionNode(array $values, ObjectType|InputObjectType $type, ValidationNode $parent): array { @@ -278,11 +264,11 @@ private function createCollectionNode(array $values, ObjectType|InputObjectType private function createObjectNode(array $value, ObjectType|InputObjectType $type, ValidationNode $parent): ValidationNode { /** @phpstan-ignore-next-line */ - $classValidation = static::normalizeConfig($type->config['validation'] ?? []); + $classValidation = Utils::normalizeConfig($type->config['validation'] ?? []); return $this->buildValidationTree( new ValidationNode($type, null, $parent, $this->resolverArgs), - self::unclosure($type->config['fields']), + Utils::unclosure($type->config['fields']), $classValidation, $value ); @@ -290,7 +276,7 @@ private function createObjectNode(array $value, ObjectType|InputObjectType $type private function applyClassValidation(ObjectMetadata $metadata, array $rules): void { - $rules = static::normalizeConfig($rules); + $rules = Utils::normalizeConfig($rules); foreach ($rules as $key => $value) { switch ($key) { @@ -299,7 +285,7 @@ private function applyClassValidation(ObjectMetadata $metadata, array $rules): v $metadata->addConstraints($linkedMetadata->getConstraints()); break; case 'constraints': - foreach ($this->unclosure($value) as $constraint) { + foreach (Utils::unclosure($value) as $constraint) { if ($constraint instanceof Constraint) { $metadata->addConstraint($constraint); } elseif ($constraint instanceof GroupSequence) { @@ -312,48 +298,9 @@ private function applyClassValidation(ObjectMetadata $metadata, array $rules): v } /** - * Restructures short forms into the full form array and - * unwraps constraints in closures. - * - * @param mixed $config - */ - public static function normalizeConfig($config): array - { - if ($config instanceof Closure) { - return ['constraints' => $config()]; - } - - if (self::CASCADE === $config) { - return ['cascade' => []]; - } - - if (isset($config['constraints']) && $config['constraints'] instanceof Closure) { - $config['constraints'] = $config['constraints'](); - } - - return $config; - } - - /** - * @param mixed $value - * - * @return mixed - */ - private function unclosure($value) - { - if ($value instanceof Closure) { - return $value(); - } - - return $value; - } - - /** - * @param string|array|null $groups - * * @throws ArgumentsValidationException */ - public function __invoke($groups = null, bool $throw = true): ?ConstraintViolationListInterface + public function __invoke(array|string|null $groups = null, bool $throw = true): ?ConstraintViolationListInterface { return $this->validate($groups, $throw); } diff --git a/src/Validator/Utils.php b/src/Validator/Utils.php new file mode 100644 index 000000000..3773c4838 --- /dev/null +++ b/src/Validator/Utils.php @@ -0,0 +1,86 @@ + $config()]; + } + + if (InputValidator::CASCADE === $config) { + return ['cascade' => []]; + } + + if (isset($config['constraints']) && $config['constraints'] instanceof Closure) { + $config['constraints'] = $config['constraints'](); + } + + return $config; + } + + /** + * @param mixed $value + * @return mixed + */ + public static function unclosure($value) + { + if ($value instanceof Closure) { + return $value(); + } + + return $value; + } + + public static function isListOfType(GeneratedTypeInterface|ListOfType|NonNull $type): bool + { + if ($type instanceof ListOfType || ($type instanceof NonNull && $type->getWrappedType() instanceof ListOfType)) { + return true; + } + + return false; + } + + /** + * Since all GraphQL arguments and fields are represented by ValidationNode + * objects, it is possible to define constraints at the class level. + * + * Class level constraints can be defined in three different ways: + * - linked from an existing class/entity + * - defined per field + * - defined per type + * + * This method merges all of them into a single array and returns it. + * + * @link https://github.com/overblog/GraphQLBundle/blob/master/docs/validation/index.md#applying-of-validation-constraints + */ + public static function getClassLevelConstraints(ResolveInfo $info): array + { + $typeLevel = Utils::normalizeConfig($info->parentType->config['validation'] ?? []); + $fieldLevel = Utils::normalizeConfig($info->fieldDefinition->config['validation'] ?? []); + + return array_filter([ + 'link' => $fieldLevel['link'] ?? $typeLevel['link'] ?? null, + 'constraints' => [ + ...($typeLevel['constraints'] ?? []), + ...($fieldLevel['constraints'] ?? []), + ], + ]); + } +} From fd372ac1ff057e5b98db4a4fca07a25079b72f27 Mon Sep 17 00:00:00 2001 From: murtukov Date: Wed, 5 Nov 2025 21:03:12 +0100 Subject: [PATCH 5/7] Inline utility methods from `Utils` into `InputValidator`; remove `Utils` class. --- src/Validator/InputValidator.php | 96 ++++++++++++++++++++++++++++---- src/Validator/Utils.php | 86 ---------------------------- 2 files changed, 86 insertions(+), 96 deletions(-) delete mode 100644 src/Validator/Utils.php diff --git a/src/Validator/InputValidator.php b/src/Validator/InputValidator.php index a5ae2714b..dd578e53f 100644 --- a/src/Validator/InputValidator.php +++ b/src/Validator/InputValidator.php @@ -4,11 +4,15 @@ namespace Overblog\GraphQLBundle\Validator; +use Closure; use GraphQL\Type\Definition\InputObjectType; +use GraphQL\Type\Definition\ListOfType; +use GraphQL\Type\Definition\NonNull; use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Type\Definition\Type; use Overblog\GraphQLBundle\Definition\ResolverArgs; +use Overblog\GraphQLBundle\Definition\Type\GeneratedTypeInterface; use Overblog\GraphQLBundle\Validator\Exception\ArgumentsValidationException; use Overblog\GraphQLBundle\Validator\Mapping\MetadataFactory; use Overblog\GraphQLBundle\Validator\Mapping\ObjectMetadata; @@ -30,7 +34,7 @@ final class InputValidator { private const TYPE_PROPERTY = 'property'; private const TYPE_GETTER = 'getter'; - public const CASCADE = 'cascade'; + private const CASCADE = 'cascade'; private ResolverArgs $resolverArgs; private ValidatorInterface $defaultValidator; @@ -73,7 +77,7 @@ public function validate(string|array|null $groups = null, bool $throw = true): $this->buildValidationTree( $rootNode, $this->info->fieldDefinition->config['args'] ?? [], - Utils::getClassLevelConstraints($this->info), + $this->getClassLevelConstraints(), $this->resolverArgs->args->getArrayCopy() ); @@ -139,7 +143,7 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field foreach ($fields as $name => $arg) { $property = $arg['name'] ?? $name; - $config = Utils::normalizeConfig($arg['validation'] ?? []); + $config = self::normalizeConfig($arg['validation'] ?? []); if ($this->shouldCascade($config, $inputData, $property)) { $this->handleCascade($rootObject, $property, $arg, $config, $inputData[$property]); @@ -153,7 +157,7 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field continue; } - $this->applyPropertyConstraints($metadata, $property, Utils::normalizeConfig($config)); + $this->applyPropertyConstraints($metadata, $property, self::normalizeConfig($config)); } return $rootObject; @@ -170,11 +174,11 @@ private function shouldCascade(array $config, array $inputData, string|int $prop */ private function handleCascade(ValidationNode $rootObject, string|int $property, array $arg, array $config, mixed $value): void { - $argType = Utils::unclosure($arg['type']); + $argType = self::unclosure($arg['type']); /** @var ObjectType|InputObjectType $type */ $type = Type::getNamedType($argType); - if (Utils::isListOfType($argType)) { + if (self::isListOfType($argType)) { $rootObject->$property = $this->createCollectionNode($value, $type, $rootObject); } else { $rootObject->$property = $this->createObjectNode($value, $type, $rootObject); @@ -264,11 +268,11 @@ private function createCollectionNode(array $values, ObjectType|InputObjectType private function createObjectNode(array $value, ObjectType|InputObjectType $type, ValidationNode $parent): ValidationNode { /** @phpstan-ignore-next-line */ - $classValidation = Utils::normalizeConfig($type->config['validation'] ?? []); + $classValidation = self::normalizeConfig($type->config['validation'] ?? []); return $this->buildValidationTree( new ValidationNode($type, null, $parent, $this->resolverArgs), - Utils::unclosure($type->config['fields']), + self::unclosure($type->config['fields']), $classValidation, $value ); @@ -276,7 +280,7 @@ private function createObjectNode(array $value, ObjectType|InputObjectType $type private function applyClassValidation(ObjectMetadata $metadata, array $rules): void { - $rules = Utils::normalizeConfig($rules); + $rules = self::normalizeConfig($rules); foreach ($rules as $key => $value) { switch ($key) { @@ -285,7 +289,7 @@ private function applyClassValidation(ObjectMetadata $metadata, array $rules): v $metadata->addConstraints($linkedMetadata->getConstraints()); break; case 'constraints': - foreach (Utils::unclosure($value) as $constraint) { + foreach (self::unclosure($value) as $constraint) { if ($constraint instanceof Constraint) { $metadata->addConstraint($constraint); } elseif ($constraint instanceof GroupSequence) { @@ -297,6 +301,78 @@ private function applyClassValidation(ObjectMetadata $metadata, array $rules): v } } + /** + * Since all GraphQL arguments and fields are represented by ValidationNode + * objects, it is possible to define constraints at the class level. + * + * Class level constraints can be defined in three different ways: + * - linked from an existing class/entity + * - defined per field + * - defined per type + * + * This method merges all of them into a single array and returns it. + * + * @link https://github.com/overblog/GraphQLBundle/blob/master/docs/validation/index.md#applying-of-validation-constraints + */ + private function getClassLevelConstraints(): array + { + $typeLevel = self::normalizeConfig($this->info->parentType->config['validation'] ?? []); + $fieldLevel = self::normalizeConfig($this->info->fieldDefinition->config['validation'] ?? []); + + return array_filter([ + 'link' => $fieldLevel['link'] ?? $typeLevel['link'] ?? null, + 'constraints' => [ + ...($typeLevel['constraints'] ?? []), + ...($fieldLevel['constraints'] ?? []), + ], + ]); + } + + /** + * Restructures short forms into the full form array and + * unwraps constraints in closures. + * + * @param Closure $config + */ + private static function normalizeConfig(mixed $config): array + { + if ($config instanceof Closure) { + return ['constraints' => $config()]; + } + + if (InputValidator::CASCADE === $config) { + return ['cascade' => []]; + } + + if (isset($config['constraints']) && $config['constraints'] instanceof Closure) { + $config['constraints'] = $config['constraints'](); + } + + return $config; + } + + /** + * @param mixed $value + * @return mixed + */ + private static function unclosure($value) + { + if ($value instanceof Closure) { + return $value(); + } + + return $value; + } + + private static function isListOfType(GeneratedTypeInterface|ListOfType|NonNull $type): bool + { + if ($type instanceof ListOfType || ($type instanceof NonNull && $type->getWrappedType() instanceof ListOfType)) { + return true; + } + + return false; + } + /** * @throws ArgumentsValidationException */ diff --git a/src/Validator/Utils.php b/src/Validator/Utils.php deleted file mode 100644 index 3773c4838..000000000 --- a/src/Validator/Utils.php +++ /dev/null @@ -1,86 +0,0 @@ - $config()]; - } - - if (InputValidator::CASCADE === $config) { - return ['cascade' => []]; - } - - if (isset($config['constraints']) && $config['constraints'] instanceof Closure) { - $config['constraints'] = $config['constraints'](); - } - - return $config; - } - - /** - * @param mixed $value - * @return mixed - */ - public static function unclosure($value) - { - if ($value instanceof Closure) { - return $value(); - } - - return $value; - } - - public static function isListOfType(GeneratedTypeInterface|ListOfType|NonNull $type): bool - { - if ($type instanceof ListOfType || ($type instanceof NonNull && $type->getWrappedType() instanceof ListOfType)) { - return true; - } - - return false; - } - - /** - * Since all GraphQL arguments and fields are represented by ValidationNode - * objects, it is possible to define constraints at the class level. - * - * Class level constraints can be defined in three different ways: - * - linked from an existing class/entity - * - defined per field - * - defined per type - * - * This method merges all of them into a single array and returns it. - * - * @link https://github.com/overblog/GraphQLBundle/blob/master/docs/validation/index.md#applying-of-validation-constraints - */ - public static function getClassLevelConstraints(ResolveInfo $info): array - { - $typeLevel = Utils::normalizeConfig($info->parentType->config['validation'] ?? []); - $fieldLevel = Utils::normalizeConfig($info->fieldDefinition->config['validation'] ?? []); - - return array_filter([ - 'link' => $fieldLevel['link'] ?? $typeLevel['link'] ?? null, - 'constraints' => [ - ...($typeLevel['constraints'] ?? []), - ...($fieldLevel['constraints'] ?? []), - ], - ]); - } -} From f9b82f6e531c511c1733e10c8511766ffbca9455 Mon Sep 17 00:00:00 2001 From: murtukov Date: Thu, 6 Nov 2025 02:27:27 +0100 Subject: [PATCH 6/7] Refactor `InputValidator` methods * Rename `applyClassValidation` to `applyClassConstraints`. * Enhance docblocks for readability and clarity. * Fix type hints and add return type for `unclosure` method. * Update inline comments for better explanation of logic. --- src/Validator/InputValidator.php | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/Validator/InputValidator.php b/src/Validator/InputValidator.php index dd578e53f..3005a93a0 100644 --- a/src/Validator/InputValidator.php +++ b/src/Validator/InputValidator.php @@ -112,8 +112,8 @@ private function createValidator(MetadataFactory $metadataFactory): ValidatorInt } /** - * Either returns an existing metadata object related to - * the ValidationNode object or creates a new one. + * Either returns an existing metadata object related to the ValidationNode + * object or creates a new one. */ private function getMetadata(ValidationNode $rootObject): ObjectMetadata { @@ -130,15 +130,15 @@ private function getMetadata(ValidationNode $rootObject): ObjectMetadata } /** - * Creates a map of ValidationNode objects from args and - * simultaneously applies validation constraints to them. + * Creates a map of ValidationNode objects from args and simultaneously + * applies validation constraints to them. */ private function buildValidationTree(ValidationNode $rootObject, iterable $fields, array $classValidation, array $inputData): ValidationNode { $metadata = $this->getMetadata($rootObject); if (!empty($classValidation)) { - $this->applyClassValidation($metadata, $classValidation); + $this->applyClassConstraints($metadata, $classValidation); } foreach ($fields as $name => $arg) { @@ -169,12 +169,13 @@ private function shouldCascade(array $config, array $inputData, string|int $prop } /** - * Creates a new nested ValidationNode object or a collection of them - * based on the type of the argument and applies the cascade validation. + * Creates a new nested ValidationNode object or a collection of them based + * on the type of the argument and applies the cascade validation. */ private function handleCascade(ValidationNode $rootObject, string|int $property, array $arg, array $config, mixed $value): void { $argType = self::unclosure($arg['type']); + /** @var ObjectType|InputObjectType $type */ $type = Type::getNamedType($argType); @@ -278,7 +279,7 @@ private function createObjectNode(array $value, ObjectType|InputObjectType $type ); } - private function applyClassValidation(ObjectMetadata $metadata, array $rules): void + private function applyClassConstraints(ObjectMetadata $metadata, array $rules): void { $rules = self::normalizeConfig($rules); @@ -306,7 +307,7 @@ private function applyClassValidation(ObjectMetadata $metadata, array $rules): v * objects, it is possible to define constraints at the class level. * * Class level constraints can be defined in three different ways: - * - linked from an existing class/entity + * - linked to an existing class/entity * - defined per field * - defined per type * @@ -329,8 +330,8 @@ private function getClassLevelConstraints(): array } /** - * Restructures short forms into the full form array and - * unwraps constraints in closures. + * Restructures short forms into the full form array and unwraps + * constraints in closures. * * @param Closure $config */ @@ -355,7 +356,7 @@ private static function normalizeConfig(mixed $config): array * @param mixed $value * @return mixed */ - private static function unclosure($value) + private static function unclosure($value): mixed { if ($value instanceof Closure) { return $value(); From 460efc624b9ce7a625006caf8f46d9bc91fc03a7 Mon Sep 17 00:00:00 2001 From: murtukov Date: Fri, 7 Nov 2025 02:18:17 +0100 Subject: [PATCH 7/7] Make `CASCADE` constant public in `InputValidator` --- src/Validator/InputValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Validator/InputValidator.php b/src/Validator/InputValidator.php index 3005a93a0..8bb53751b 100644 --- a/src/Validator/InputValidator.php +++ b/src/Validator/InputValidator.php @@ -34,7 +34,7 @@ final class InputValidator { private const TYPE_PROPERTY = 'property'; private const TYPE_GETTER = 'getter'; - private const CASCADE = 'cascade'; + public const CASCADE = 'cascade'; private ResolverArgs $resolverArgs; private ValidatorInterface $defaultValidator;