-
Notifications
You must be signed in to change notification settings - Fork 223
Fix input-object validation issue
#1215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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); | ||
|
|
@@ -141,7 +155,6 @@ private function buildValidationTree(ValidationNode $rootObject, iterable $field | |
| $config = static::normalizeConfig($arg['validation'] ?? []); | ||
|
|
||
| if (isset($config['cascade']) && isset($inputData[$property])) { | ||
| $groups = $config['cascade']; | ||
| $argType = $this->unclosure($arg['type']); | ||
|
|
||
| /** @var ObjectType|InputObjectType $type */ | ||
|
|
@@ -154,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; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to proceed and add any more constraints if it was set to |
||
| } else { | ||
| $rootObject->$property = $inputData[$property] ?? null; | ||
| } | ||
|
|
||
| if ($metadata->hasPropertyMetadata($property)) { | ||
| continue; | ||
| } | ||
|
Comment on lines
+186
to
+188
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If constraints were set for the property, don't add them the second time |
||
|
|
||
| $config = static::normalizeConfig($config); | ||
|
|
||
| // Apply validation constraints for the property | ||
| foreach ($config as $key => $value) { | ||
| switch ($key) { | ||
| case 'link': | ||
|
|
@@ -195,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; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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'] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -377,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 | ||
| } | ||
| ] | ||
| ) | ||
| } | ||
| '; | ||
|
Comment on lines
+383
to
+422
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A new test case to cover scenarios when input objects ( |
||
|
|
||
| $result = $this->executeGraphQLRequest($query); | ||
| $this->assertSame(ExpectedErrors::PARTIAL_INPUT_OBJECTS_COLLECTION, $result['errors'][0]); | ||
| $this->assertNull($result['data']['partialInputObjectsCollectionValidation']); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method reuses metadata if it was already created for the input-object of the same type