diff --git a/src/Validator/InputValidator.php b/src/Validator/InputValidator.php index 8059456e..8bb53751 100644 --- a/src/Validator/InputValidator.php +++ b/src/Validator/InputValidator.php @@ -61,6 +61,8 @@ public function __construct( } /** + * Entry point. + * * @throws ArgumentsValidationException */ public function validate(string|array|null $groups = null, bool $throw = true): ?ConstraintViolationListInterface @@ -72,17 +74,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, + $this->getClassLevelConstraints(), $this->resolverArgs->args->getArrayCopy() ); $validator = $this->createValidator($this->metadataFactory); - $errors = $validator->validate($rootNode, null, $groups); if ($throw && $errors->count() > 0) { @@ -92,22 +91,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() @@ -125,104 +112,149 @@ private function createValidator(MetadataFactory $metadataFactory): ValidatorInt } /** - * Creates a composition of ValidationNode objects from args - * and simultaneously applies to them validation constraints. + * Either returns an existing metadata object related to the ValidationNode + * object or creates a new one. */ - private function buildValidationTree(ValidationNode $rootObject, iterable $fields, array $classValidation, array $inputData): ValidationNode + 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 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) { $property = $arg['name'] ?? $name; - $config = static::normalizeConfig($arg['validation'] ?? []); + $config = self::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 ($this->shouldCascade($config, $inputData, $property)) { + $this->handleCascade($rootObject, $property, $arg, $config, $inputData[$property]); + continue; // delegated to nested object } - if (isset($config['cascade']) && isset($inputData[$property])) { - $groups = $config['cascade']; - $argType = $this->unclosure($arg['type']); - - /** @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; + } - $valid = new Valid(); + $this->applyPropertyConstraints($metadata, $property, self::normalizeConfig($config)); + } - if (!empty($groups)) { - $valid->groups = $groups; - } + return $rootObject; + } - $metadata->addPropertyConstraint($property, $valid); - } else { - $rootObject->$property = $inputData[$property] ?? null; - } + private function shouldCascade(array $config, array $inputData, string|int $property): bool + { + return isset($config['cascade']) && isset($inputData[$property]); + } - $config = static::normalizeConfig($config); + /** + * 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']); - foreach ($config as $key => $value) { - switch ($key) { - case 'link': - [$fqcn, $classProperty, $type] = $value; + /** @var ObjectType|InputObjectType $type */ + $type = Type::getNamedType($argType); - if (!in_array($fqcn, $this->cachedMetadata)) { - /** @phpstan-ignore-next-line */ - $this->cachedMetadata[$fqcn] = $this->defaultValidator->getMetadataFor($fqcn); - } + if (self::isListOfType($argType)) { + $rootObject->$property = $this->createCollectionNode($value, $type, $rootObject); + } else { + $rootObject->$property = $this->createObjectNode($value, $type, $rootObject); + } - // 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()); - } + // Mark the property for recursive validation + $this->addValidConstraint($this->getMetadata($rootObject), (string) $property, $config['cascade']); + } - break; - case 'constraints': - $metadata->addPropertyConstraints($property, $value); - break; - case 'cascade': - break; - } - } + /** + * 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; } - $this->metadataFactory->addMetadata($metadata); + $metadata->addPropertyConstraint($property, $valid); + } - return $rootObject; + /** + * 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': + // Add constraints from the linked class + $this->addLinkedConstraints((string) $property, $value, $metadata); + break; + case 'constraints': + // Add constraints from the yml config directly + $metadata->addPropertyConstraints((string) $property, $value); + break; + case 'cascade': + // Cascade validation was already handled recursively. + break; + } + } } - private static function isListOfType(GeneratedTypeInterface|ListOfType|NonNull $type): bool + private function addLinkedConstraints(string $property, array $link, ObjectMetadata $metadata, ): void { - if ($type instanceof ListOfType || ($type instanceof NonNull && $type->getWrappedType() instanceof ListOfType)) { - return true; + [$fqcn, $classProperty, $type] = $link; + + if (!in_array($fqcn, $this->cachedMetadata)) { + /** @phpstan-ignore-next-line */ + $this->cachedMetadata[$fqcn] = $this->defaultValidator->getMetadataFor($fqcn); } - return false; + // Get metadata from the property and its 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 createCollectionNode(array $values, ObjectType|InputObjectType $type, ValidationNode $parent): array { $collection = []; @@ -237,7 +269,7 @@ 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 = self::normalizeConfig($type->config['validation'] ?? []); return $this->buildValidationTree( new ValidationNode($type, null, $parent, $this->resolverArgs), @@ -247,9 +279,9 @@ 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 = static::normalizeConfig($rules); + $rules = self::normalizeConfig($rules); foreach ($rules as $key => $value) { switch ($key) { @@ -258,7 +290,7 @@ private function applyClassValidation(ObjectMetadata $metadata, array $rules): v $metadata->addConstraints($linkedMetadata->getConstraints()); break; case 'constraints': - foreach ($this->unclosure($value) as $constraint) { + foreach (self::unclosure($value) as $constraint) { if ($constraint instanceof Constraint) { $metadata->addConstraint($constraint); } elseif ($constraint instanceof GroupSequence) { @@ -271,18 +303,45 @@ private function applyClassValidation(ObjectMetadata $metadata, array $rules): v } /** - * Restructures short forms into the full form array and - * unwraps constraints in closures. + * 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 to an existing class/entity + * - defined per field + * - defined per type * - * @param mixed $config + * 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 normalizeConfig($config): array + 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 (self::CASCADE === $config) { + if (InputValidator::CASCADE === $config) { return ['cascade' => []]; } @@ -295,10 +354,9 @@ public static function normalizeConfig($config): array /** * @param mixed $value - * * @return mixed */ - private function unclosure($value) + private static function unclosure($value): mixed { if ($value instanceof Closure) { return $value(); @@ -307,12 +365,19 @@ private function unclosure($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; + } + /** - * @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/tests/Functional/App/config/validator/mapping/Address.types.yml b/tests/Functional/App/config/validator/mapping/Address.types.yml index 3426bd40..1e3a6c34 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 00000000..09f2f266 --- /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 e320f09a..6c85fd67 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 909315b7..55750655 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 bb8e2964..00000000 --- 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 e4f5b34d..5aef2fb9 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 f454ab05..7deae5ee 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']); + } }