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

View File

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

View File

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

View File

@ -12,12 +12,19 @@
namespace Symfony\Component\Form\Tests\Extension\Validator;
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\ValidatorExtension;
use Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser;
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\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\Tests\Fixtures\FakeMetadataFactory;
use Symfony\Component\Validator\Validation;
@ -45,4 +52,78 @@ class ValidatorExtensionTest extends TestCase
$this->assertSame(CascadingStrategy::CASCADE, $metadata->getPropertyMetadata('children')[0]->cascadingStrategy);
$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');
}
public function testAbortMappingIfNotSubmitted()
public function testMappingIfNotSubmitted()
{
$violation = $this->getConstraintViolation('children[address].data.street');
$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, $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');
$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, $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()