do not validate non-submitted form fields in PATCH requests

This commit is contained in:
Christian Flothmann 2019-02-16 00:03:48 +01:00
parent 823ccf086d
commit a60d802619
5 changed files with 126 additions and 51 deletions

View File

@ -41,7 +41,7 @@ class FormValidator extends ConstraintValidator
$validator = $this->context->getValidator()->inContext($this->context); $validator = $this->context->getValidator()->inContext($this->context);
if ($form->isSynchronized()) { if ($form->isSubmitted() && $form->isSynchronized()) {
// Validate the form data only if transformation succeeded // Validate the form data only if transformation succeeded
$groups = self::getValidationGroups($form); $groups = self::getValidationGroups($form);
$data = $form->getData(); $data = $form->getData();
@ -90,7 +90,7 @@ class FormValidator extends ConstraintValidator
} }
} }
} }
} else { } elseif (!$form->isSynchronized()) {
$childrenSynchronized = true; $childrenSynchronized = true;
/** @var FormInterface $child */ /** @var FormInterface $child */

View File

@ -273,9 +273,6 @@ class ViolationMapper implements ViolationMapperInterface
*/ */
private function acceptsErrors(FormInterface $form) private function acceptsErrors(FormInterface $form)
{ {
// Ignore non-submitted forms. This happens, for example, in PATCH return $this->allowNonSynchronized || $form->isSynchronized();
// requests.
// https://github.com/symfony/symfony/pull/10567
return $form->isSubmitted() && ($this->allowNonSynchronized || $form->isSynchronized());
} }
} }

View File

@ -61,9 +61,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
{ {
$object = new \stdClass(); $object = new \stdClass();
$options = ['validation_groups' => ['group1', 'group2']]; $options = ['validation_groups' => ['group1', 'group2']];
$form = $this->getBuilder('name', '\stdClass', $options) $form = $this->getCompoundForm($object, $options);
->setData($object) $form->submit([]);
->getForm();
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@ -82,9 +81,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
'validation_groups' => ['group1', 'group2'], 'validation_groups' => ['group1', 'group2'],
'constraints' => [$constraint1, $constraint2], 'constraints' => [$constraint1, $constraint2],
]; ];
$form = $this->getBuilder('name', '\stdClass', $options) $form = $this->getCompoundForm($object, $options);
->setData($object) $form->submit([]);
->getForm();
// First default constraints // First default constraints
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@ -110,10 +108,9 @@ class FormValidatorTest extends ConstraintValidatorTestCase
'validation_groups' => ['group1', 'group2'], 'validation_groups' => ['group1', 'group2'],
'constraints' => [new Valid()], 'constraints' => [new Valid()],
]; ];
$form = $this->getBuilder('name', '\stdClass', $options)->getForm(); $form = $this->getCompoundForm($object, $options);
$parent->add($form); $parent->add($form);
$parent->submit([]);
$form->setData($object);
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@ -146,8 +143,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
public function testMissingConstraintIndex() public function testMissingConstraintIndex()
{ {
$object = new \stdClass(); $object = new \stdClass();
$form = new FormBuilder('name', '\stdClass', $this->dispatcher, $this->factory); $form = $this->getCompoundForm($object);
$form = $form->setData($object)->getForm(); $form->submit([]);
$this->expectValidateAt(0, 'data', $object, ['Default']); $this->expectValidateAt(0, 'data', $object, ['Default']);
@ -170,10 +167,9 @@ class FormValidatorTest extends ConstraintValidatorTestCase
'validation_groups' => ['group1', 'group2'], 'validation_groups' => ['group1', 'group2'],
'constraints' => [$constraint1, $constraint2], 'constraints' => [$constraint1, $constraint2],
]; ];
$form = $this->getBuilder('name', '\stdClass', $options) $form = $this->getCompoundForm($object, $options);
->setData($object)
->getForm();
$parent->add($form); $parent->add($form);
$parent->submit([]);
$this->expectValidateValueAt(0, 'data', $object, $constraint1, 'group1'); $this->expectValidateValueAt(0, 'data', $object, $constraint1, 'group1');
$this->expectValidateValueAt(1, 'data', $object, $constraint2, 'group2'); $this->expectValidateValueAt(1, 'data', $object, $constraint2, 'group2');
@ -361,9 +357,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
{ {
$object = new \stdClass(); $object = new \stdClass();
$options = ['validation_groups' => new GroupSequence(['group1', 'group2'])]; $options = ['validation_groups' => new GroupSequence(['group1', 'group2'])];
$form = $this->getBuilder('name', '\stdClass', $options) $form = $this->getCompoundForm($object, $options);
->setData($object) $form->submit([]);
->getForm();
$this->expectValidateAt(0, 'data', $object, new GroupSequence(['group1', 'group2'])); $this->expectValidateAt(0, 'data', $object, new GroupSequence(['group1', 'group2']));
$this->expectValidateAt(1, 'data', $object, new GroupSequence(['group1', 'group2'])); $this->expectValidateAt(1, 'data', $object, new GroupSequence(['group1', 'group2']));
@ -377,9 +372,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
{ {
$object = new \stdClass(); $object = new \stdClass();
$options = ['validation_groups' => [$this, 'getValidationGroups']]; $options = ['validation_groups' => [$this, 'getValidationGroups']];
$form = $this->getBuilder('name', '\stdClass', $options) $form = $this->getCompoundForm($object, $options);
->setData($object) $form->submit([]);
->getForm();
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@ -392,9 +386,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
{ {
$object = new \stdClass(); $object = new \stdClass();
$options = ['validation_groups' => 'header']; $options = ['validation_groups' => 'header'];
$form = $this->getBuilder('name', '\stdClass', $options) $form = $this->getCompoundForm($object, $options);
->setData($object) $form->submit([]);
->getForm();
$this->expectValidateAt(0, 'data', $object, ['header']); $this->expectValidateAt(0, 'data', $object, ['header']);
@ -409,9 +402,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
$options = ['validation_groups' => function (FormInterface $form) { $options = ['validation_groups' => function (FormInterface $form) {
return ['group1', 'group2']; return ['group1', 'group2'];
}]; }];
$form = $this->getBuilder('name', '\stdClass', $options) $form = $this->getCompoundForm($object, $options);
->setData($object) $form->submit([]);
->getForm();
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@ -455,7 +447,7 @@ class FormValidatorTest extends ConstraintValidatorTestCase
->setCompound(true) ->setCompound(true)
->setDataMapper(new PropertyPathMapper()) ->setDataMapper(new PropertyPathMapper())
->getForm(); ->getForm();
$form = $this->getForm('name', '\stdClass', [ $form = $this->getCompoundForm($object, [
'validation_groups' => 'form_group', 'validation_groups' => 'form_group',
'constraints' => [new Valid()], 'constraints' => [new Valid()],
]); ]);
@ -465,7 +457,7 @@ class FormValidatorTest extends ConstraintValidatorTestCase
'validation_groups' => 'button_group', 'validation_groups' => 'button_group',
])); ]));
$form->setData($object); $parent->submit([]);
$this->expectValidateAt(0, 'data', $object, ['form_group']); $this->expectValidateAt(0, 'data', $object, ['form_group']);
@ -484,10 +476,9 @@ class FormValidatorTest extends ConstraintValidatorTestCase
->setDataMapper(new PropertyPathMapper()) ->setDataMapper(new PropertyPathMapper())
->getForm(); ->getForm();
$formOptions = ['constraints' => [new Valid()]]; $formOptions = ['constraints' => [new Valid()]];
$form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm(); $form = $this->getCompoundForm($object, $formOptions);
$parent->add($form); $parent->add($form);
$parent->submit([]);
$form->setData($object);
$this->expectValidateAt(0, 'data', $object, ['group']); $this->expectValidateAt(0, 'data', $object, ['group']);
@ -506,10 +497,9 @@ class FormValidatorTest extends ConstraintValidatorTestCase
->setDataMapper(new PropertyPathMapper()) ->setDataMapper(new PropertyPathMapper())
->getForm(); ->getForm();
$formOptions = ['constraints' => [new Valid()]]; $formOptions = ['constraints' => [new Valid()]];
$form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm(); $form = $this->getCompoundForm($object, $formOptions);
$parent->add($form); $parent->add($form);
$parent->submit([]);
$form->setData($object);
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@ -523,7 +513,7 @@ class FormValidatorTest extends ConstraintValidatorTestCase
$object = new \stdClass(); $object = new \stdClass();
$parentOptions = [ $parentOptions = [
'validation_groups' => function (FormInterface $form) { 'validation_groups' => function () {
return ['group1', 'group2']; return ['group1', 'group2'];
}, },
]; ];
@ -532,10 +522,9 @@ class FormValidatorTest extends ConstraintValidatorTestCase
->setDataMapper(new PropertyPathMapper()) ->setDataMapper(new PropertyPathMapper())
->getForm(); ->getForm();
$formOptions = ['constraints' => [new Valid()]]; $formOptions = ['constraints' => [new Valid()]];
$form = $this->getBuilder('name', '\stdClass', $formOptions)->getForm(); $form = $this->getCompoundForm($object, $formOptions);
$parent->add($form); $parent->add($form);
$parent->submit([]);
$form->setData($object);
$this->expectValidateAt(0, 'data', $object, ['group1', 'group2']); $this->expectValidateAt(0, 'data', $object, ['group1', 'group2']);
@ -547,9 +536,8 @@ class FormValidatorTest extends ConstraintValidatorTestCase
public function testAppendPropertyPath() public function testAppendPropertyPath()
{ {
$object = new \stdClass(); $object = new \stdClass();
$form = $this->getBuilder('name', '\stdClass') $form = $this->getCompoundForm($object);
->setData($object) $form->submit([]);
->getForm();
$this->expectValidateAt(0, 'data', $object, ['Default']); $this->expectValidateAt(0, 'data', $object, ['Default']);
@ -690,6 +678,15 @@ class FormValidatorTest extends ConstraintValidatorTestCase
return $this->getBuilder($name, $dataClass, $options)->getForm(); return $this->getBuilder($name, $dataClass, $options)->getForm();
} }
private function getCompoundForm($data, array $options = [])
{
return $this->getBuilder('name', \get_class($data), $options)
->setData($data)
->setCompound(true)
->setDataMapper(new PropertyPathMapper())
->getForm();
}
private function getSubmitButton($name = 'name', array $options = []) private function getSubmitButton($name = 'name', array $options = [])
{ {
$builder = new SubmitButtonBuilder($name, $options); $builder = new SubmitButtonBuilder($name, $options);

View File

@ -12,12 +12,19 @@
namespace Symfony\Component\Form\Tests\Extension\Validator; namespace Symfony\Component\Form\Tests\Extension\Validator;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint; use Symfony\Component\Form\Extension\Validator\Constraints\Form as FormConstraint;
use Symfony\Component\Form\Extension\Validator\ValidatorExtension; use Symfony\Component\Form\Extension\Validator\ValidatorExtension;
use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser; use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser;
use Symfony\Component\Form\Form; use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormFactoryBuilder;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\Validator\Constraints\NotBlank;
use Symfony\Component\Validator\Mapping\CascadingStrategy; use Symfony\Component\Validator\Mapping\CascadingStrategy;
use Symfony\Component\Validator\Mapping\ClassMetadata; use Symfony\Component\Validator\Mapping\ClassMetadata;
use Symfony\Component\Validator\Mapping\Factory\LazyLoadingMetadataFactory;
use Symfony\Component\Validator\Mapping\Loader\StaticMethodLoader;
use Symfony\Component\Validator\Mapping\TraversalStrategy; use Symfony\Component\Validator\Mapping\TraversalStrategy;
use Symfony\Component\Validator\Tests\Fixtures\FakeMetadataFactory; use Symfony\Component\Validator\Tests\Fixtures\FakeMetadataFactory;
use Symfony\Component\Validator\Validation; use Symfony\Component\Validator\Validation;
@ -45,4 +52,78 @@ class ValidatorExtensionTest extends TestCase
$this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy); $this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy);
$this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy); $this->assertSame(TraversalStrategy::IMPLICIT, $metadata->getPropertyMetadata('children')[0]->traversalStrategy);
} }
public function testDataConstraintsInvalidateFormEvenIfFieldIsNotSubmitted()
{
$form = $this->createForm(FooType::class);
$form->submit(['baz' => 'foobar'], false);
$this->assertTrue($form->isSubmitted());
$this->assertFalse($form->isValid());
$this->assertFalse($form->get('bar')->isSubmitted());
$this->assertCount(1, $form->get('bar')->getErrors());
}
public function testFieldConstraintsDoNotInvalidateFormIfFieldIsNotSubmitted()
{
$form = $this->createForm(FooType::class);
$form->submit(['bar' => 'foobar'], false);
$this->assertTrue($form->isSubmitted());
$this->assertTrue($form->isValid());
}
public function testFieldConstraintsInvalidateFormIfFieldIsSubmitted()
{
$form = $this->createForm(FooType::class);
$form->submit(['bar' => 'foobar', 'baz' => ''], false);
$this->assertTrue($form->isSubmitted());
$this->assertFalse($form->isValid());
$this->assertTrue($form->get('bar')->isSubmitted());
$this->assertTrue($form->get('bar')->isValid());
$this->assertTrue($form->get('baz')->isSubmitted());
$this->assertFalse($form->get('baz')->isValid());
}
private function createForm($type)
{
$validator = Validation::createValidatorBuilder()
->setMetadataFactory(new LazyLoadingMetadataFactory(new StaticMethodLoader()))
->getValidator();
$formFactoryBuilder = new FormFactoryBuilder();
$formFactoryBuilder->addExtension(new ValidatorExtension($validator));
$formFactory = $formFactoryBuilder->getFormFactory();
return $formFactory->create($type);
}
}
class Foo
{
public $bar;
public $baz;
public static function loadValidatorMetadata(ClassMetadata $metadata)
{
$metadata->addPropertyConstraint('bar', new NotBlank());
}
}
class FooType extends AbstractType
{
public function buildForm(FormBuilderInterface $builder, array $options)
{
$builder
->add('bar')
->add('baz', null, [
'constraints' => [new NotBlank()],
])
;
}
public function configureOptions(OptionsResolver $resolver)
{
$resolver->setDefault('data_class', Foo::class);
}
} }

View File

@ -205,7 +205,7 @@ class ViolationMapperTest extends TestCase
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one');
} }
public function testAbortMappingIfNotSubmitted() public function testMappingIfNotSubmitted()
{ {
$violation = $this->getConstraintViolation('children[address].data.street'); $violation = $this->getConstraintViolation('children[address].data.street');
$parent = $this->getForm('parent'); $parent = $this->getForm('parent');
@ -225,10 +225,10 @@ class ViolationMapperTest extends TestCase
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); $this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
} }
public function testAbortDotRuleMappingIfNotSubmitted() public function testDotRuleMappingIfNotSubmitted()
{ {
$violation = $this->getConstraintViolation('data.address'); $violation = $this->getConstraintViolation('data.address');
$parent = $this->getForm('parent'); $parent = $this->getForm('parent');
@ -250,7 +250,7 @@ class ViolationMapperTest extends TestCase
$this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one'); $this->assertCount(0, $parent->getErrors(), $parent->getName().' should not have an error, but has one');
$this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one'); $this->assertCount(0, $child->getErrors(), $child->getName().' should not have an error, but has one');
$this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); $this->assertCount(1, $grandChild->getErrors(), $grandChild->getName().' should have one error');
} }
public function provideDefaultTests() public function provideDefaultTests()