[Form] Added method Form::getClickedButton() to remove memory leak in FormValidator

This commit is contained in:
Bernhard Schussek 2013-11-09 16:25:41 +01:00
parent 5329ab5d5f
commit 1bb7e4dc7b
5 changed files with 132 additions and 51 deletions

View File

@ -60,7 +60,7 @@ class ButtonBuilder implements \IteratorAggregate, FormBuilderInterface
*
* @throws InvalidArgumentException If the name is empty.
*/
public function __construct($name, array $options)
public function __construct($name, array $options = array())
{
if (empty($name) && 0 != $name) {
throw new InvalidArgumentException('Buttons cannot have empty names.');

View File

@ -22,11 +22,6 @@ use Symfony\Component\Validator\ConstraintValidator;
*/
class FormValidator extends ConstraintValidator
{
/**
* @var array
*/
private static $clickedButtons = array();
/**
* @var ServerParams
*/
@ -52,10 +47,6 @@ class FormValidator extends ConstraintValidator
return;
}
// If the form was previously validated, remove it from the cache in
// case the clicked button has changed
unset(self::$clickedButtons[spl_object_hash($form)]);
/* @var FormInterface $form */
$config = $form->getConfig();
@ -181,21 +172,15 @@ class FormValidator extends ConstraintValidator
*/
private static function getValidationGroups(FormInterface $form)
{
$root = $form->getRoot();
$rootHash = spl_object_hash($root);
// Determine the clicked button of the complete form tree
if (!array_key_exists($rootHash, self::$clickedButtons)) {
// Only call findClickedButton() once to prevent an exponential
// runtime
// https://github.com/symfony/symfony/issues/8317
self::$clickedButtons[$rootHash] = self::findClickedButton($root);
$clickedButton = null;
if (method_exists($form, 'getClickedButton')) {
$clickedButton = $form->getClickedButton();
}
$button = self::$clickedButtons[$rootHash];
if (null !== $button) {
$groups = $button->getConfig()->getOption('validation_groups');
if (null !== $clickedButton) {
$groups = $clickedButton->getConfig()->getOption('validation_groups');
if (null !== $groups) {
return self::resolveValidationGroups($groups, $form);
@ -215,28 +200,6 @@ class FormValidator extends ConstraintValidator
return array(Constraint::DEFAULT_GROUP);
}
/**
* Extracts a clicked button from a form tree, if one exists.
*
* @param FormInterface $form The root form.
*
* @return ClickableInterface|null The clicked button or null.
*/
private static function findClickedButton(FormInterface $form)
{
if ($form instanceof ClickableInterface && $form->isClicked()) {
return $form;
}
foreach ($form as $child) {
if (null !== ($button = self::findClickedButton($child))) {
return $button;
}
}
return null;
}
/**
* Post-processes the validation groups option for a given form.
*

View File

@ -90,6 +90,12 @@ class Form implements \IteratorAggregate, FormInterface
*/
private $submitted = false;
/**
* The button that was used to submit the form
* @var Button
*/
private $clickedButton;
/**
* The form data in model format
* @var mixed
@ -551,6 +557,20 @@ class Form implements \IteratorAggregate, FormInterface
if (isset($submittedData[$name]) || $clearMissing) {
$child->submit(isset($submittedData[$name]) ? $submittedData[$name] : null, $clearMissing);
unset($submittedData[$name]);
if (null !== $this->clickedButton) {
continue;
}
if ($child instanceof ClickableInterface && $child->isClicked()) {
$this->clickedButton = $child;
continue;
}
if (method_exists($child, 'getClickedButton') && null !== $child->getClickedButton()) {
$this->clickedButton = $child->getClickedButton();
}
}
}
@ -730,6 +750,25 @@ class Form implements \IteratorAggregate, FormInterface
return true;
}
/**
* Returns the button that was used to submit the form.
*
* @return Button|null The clicked button or NULL if the form was not
* submitted
*/
public function getClickedButton()
{
if ($this->clickedButton) {
return $this->clickedButton;
}
if ($this->parent && method_exists($this->parent, 'getClickedButton')) {
return $this->parent->getClickedButton();
}
return null;
}
/**
* {@inheritdoc}
*/

View File

@ -13,8 +13,10 @@ namespace Symfony\Component\Form\Tests;
use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper;
use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler;
use Symfony\Component\Form\FormBuilder;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\Forms;
use Symfony\Component\Form\SubmitButtonBuilder;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\File\UploadedFile;
use Symfony\Component\Form\Tests\Fixtures\FixedDataTransformer;
@ -830,6 +832,88 @@ class CompoundFormTest extends AbstractFormTest
$this->assertEquals("name:\n ERROR: Error!\nfoo:\n No errors\n", $parent->getErrorsAsString());
}
public function testNoClickedButtonBeforeSubmission()
{
$this->assertNull($this->form->getClickedButton());
}
public function testNoClickedButton()
{
$button = $this->getMockBuilder('Symfony\Component\Form\SubmitButton')
->setConstructorArgs(array(new SubmitButtonBuilder('submit')))
->setMethods(array('isClicked'))
->getMock();
$button->expects($this->any())
->method('isClicked')
->will($this->returnValue(false));
$parentForm = $this->getBuilder('parent')->getForm();
$nestedForm = $this->getBuilder('nested')->getForm();
$this->form->setParent($parentForm);
$this->form->add($button);
$this->form->add($nestedForm);
$this->form->submit(array());
$this->assertNull($this->form->getClickedButton());
}
public function testClickedButton()
{
$button = $this->getMockBuilder('Symfony\Component\Form\SubmitButton')
->setConstructorArgs(array(new SubmitButtonBuilder('submit')))
->setMethods(array('isClicked'))
->getMock();
$button->expects($this->any())
->method('isClicked')
->will($this->returnValue(true));
$this->form->add($button);
$this->form->submit(array());
$this->assertSame($button, $this->form->getClickedButton());
}
public function testClickedButtonFromNestedForm()
{
$button = $this->getBuilder('submit')->getForm();
$nestedForm = $this->getMockBuilder('Symfony\Component\Form\Form')
->setConstructorArgs(array($this->getBuilder('nested')))
->setMethods(array('getClickedButton'))
->getMock();
$nestedForm->expects($this->any())
->method('getClickedButton')
->will($this->returnValue($button));
$this->form->add($nestedForm);
$this->form->submit(array());
$this->assertSame($button, $this->form->getClickedButton());
}
public function testClickedButtonFromParentForm()
{
$button = $this->getBuilder('submit')->getForm();
$parentForm = $this->getMockBuilder('Symfony\Component\Form\Form')
->setConstructorArgs(array($this->getBuilder('parent')))
->setMethods(array('getClickedButton'))
->getMock();
$parentForm->expects($this->any())
->method('getClickedButton')
->will($this->returnValue($button));
$this->form->setParent($parentForm);
$this->form->submit(array());
$this->assertSame($button, $this->form->getClickedButton());
}
protected function createForm()
{
return $this->getBuilder()

View File

@ -430,11 +430,11 @@ class FormValidatorTest extends \PHPUnit_Framework_TestCase
));
$parent->add($form);
$parent->add($this->getClickedSubmitButton('submit', array(
$parent->add($this->getSubmitButton('submit', array(
'validation_groups' => 'button_group',
)));
$form->setData($object);
$parent->submit(array('name' => $object, 'submit' => ''));
$context->expects($this->once())
->method('validate')
@ -733,11 +733,6 @@ class FormValidatorTest extends \PHPUnit_Framework_TestCase
return $builder->getForm();
}
private function getClickedSubmitButton($name = 'name', array $options = array())
{
return $this->getSubmitButton($name, $options)->submit('');
}
/**
* @return \PHPUnit_Framework_MockObject_MockObject
*/