8000 [Form] Reduced the number of setData() calls by deferring a Form's in… · Lumbendil/symfony@d4f4038 · GitHub
[go: up one dir, main page]
Skip to content

Commit d4f4038

Browse files
webmozartwebmozart
committed
[Form] Reduced the number of setData() calls by deferring a Form's initialization (+40ms)
1 parent 9f157a1 commit d4f4038

File tree

3 files changed

+104
-20
lines changed

3 files changed

+104
-20
lines changed

‎src/Symfony/Component/Form/Form.php

Lines changed: 86 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,26 @@ class Form implements \IteratorAggregate, FormInterface
119119
*/
120120
private $synchronized = true;
121121

122+
/**
123+
* Whether the form's data has been initialized.
124+
*
125+
* When the data is initialized with its default value, that default value
126+
* is passed through the transformer chain in order to synchronize the
127+
* model, normalized and view format for the first time. This is done
128+
* lazily in order to save performance when {@link setData()} is called
129+
* manually, making the initialization with the configured default value
130+
* superfluous.
131+
*
132+
* @var Boolean
133+
*/
134+
private $initialized = false;
135+
136+
/**
137+
* Whether setData() is currently being called.
138+
* @var Boolean
139+
*/
140+
private $lockSetData = false;
141+
122142
/**
123143
* Creates a new form based on the given configuration.
124144
*
@@ -138,8 +158,6 @@ public function __construct(FormConfigInterface $config)
138158
}
139159

140160
$this->config = $config;
141-
142-
$this->setData($config->getData());
143161
}
144162

145163
public function __clone()
@@ -327,13 +345,16 @@ public function getAttribute($name)
327345
/**
328346
* Updates the form with default data.
329347
*
330-
* @param array $modelData The data formatted as expected for the underlying object
348+
* @param mixed $modelData The data formatted as expected for the underlying object
331349
*
332350
* @return Form The current form
333351
*/
334352
public function setData($modelData)
335353
{
336-
if ($this->bound) {
354+
// If the form is bound while disabled, it is set to bound, but the data is not
355+
// changed. In such cases (i.e. when the form is not initialized yet) don't
356+
// abort this method.
357+
if ($this->bound && $this->initialized) {
337358
throw new AlreadyBoundException('You cannot change the data of a bound form');
338359
}
339360

@@ -346,6 +367,12 @@ public function setData($modelData)
346367
$modelData = clone $modelData;
347368
}
348369

370+
if ($this->lockSetData) {
371+
throw new FormException('A cycle was detected. Listeners to the PRE_SET_DATA event must not call setData(). You should call setData() on the FormEvent object instead.');
372+
}
373+
374+
$this->lockSetData = true;
375+
349376
// Hook to change content of the data
350377
$event = new FormEvent($this, $modelData);
351378
$this->config->getEventDispatcher()->dispatch(FormEvents::PRE_SET_DATA, $event);
@@ -395,8 +422,12 @@ public function setData($modelData)
395422
$this->normData = $normData;
396423
$this->viewData = $viewData;
397424
$this->synchronized = true;
425+
$this->initialized = true;
426+
$this->lockSetData = false;
398427

399-
if ($this->config->getCompound()) {
428+
// It is not necessary to invoke this method if the form doesn't have children,
429+
// even if the form is compound.
430+
if (count($this->children) > 0) {
400431
// Update child forms from the data
401432
$this->config->getDataMapper()->mapDataToForms($viewData, $this->children);
402433
}
@@ -414,16 +445,40 @@ public function setData($modelData)
414445
*/
415446
public function getData()
416447
{
448+
if (!$this->initialized) {
449+
$this->setData($this->config->getData());
450+
}
451+
417452
return $this->modelData;
418453
}
419454

455+
/**
456+
* Returns the normalized data of the form.
457+
*
458+
* @return mixed When the form is not bound, the default data is returned.
459+
* When the form is bound, the normalized bound data is
460+
* returned if the form is valid, null otherwise.
461+
*/
462+
public function getNormData()
463+
{
464+
if (!$this->initialized) {
465+
$this->setData($this->config->getData());
466+
}
467+
468+
return $this->normData;
469+
}
470+
420471
/**
421472
* Returns the data transformed by the value transformer.
422473
*
423474
* @return string
424475
*/
425476
public function getViewData()
426477
{
478+
if (!$this->initialized) {
479+
$this->setData($this->config->getData());
480+
}
481+
427482
return $this->viewData;
428483
}
429484

@@ -543,7 +598,9 @@ public function bind($submittedData)
543598
}
544599

545600
// Merge form data from children into existing view data
546-
if ($this->config->getCompound()) {
601+
// It is not necessary to invoke this method if the form has no children,
602+
// even if it is compound.
603+
if (count($this->children) > 0) {
547604
$this->config->getDataMapper()->mapFormsToData($this->children, $viewData);
548605
}
549606

@@ -573,6 +630,7 @@ public function bind($submittedData)
573630
$this->viewData = $viewData;
574631
$this->extraData = $extraData;
575632
$this->synchronized = $synchronized;
633+
$this->initialized = true;
576634

577635
$event = new FormEvent($this, $viewData);
578636
$this->config->getEventDispatcher()->dispatch(FormEvents::POST_BIND, $event);
@@ -604,18 +662,6 @@ public function bindRequest(Request $request)
604662
return $this->bind($request);
605663
}
606664

607-
/**
608-
* Returns the normalized data of the form.
609-
*
610-
* @return mixed When the form is not bound, the default data is returned.
611-
* When the form is bound, the normalized bound data is
612-
* returned if the form is valid, null otherwise.
613-
*/
614-
public function getNormData()
10000 615-
{
616-
return $this->normData;
617-
}
618-
619665
/**
620666
* Adds an error to this form.
621667
*
@@ -833,11 +879,32 @@ public function add(FormInterface $child)
833879
throw new FormException('You cannot add children to a simple form. Maybe you should set the option "compound" to true?');
834880
}
835881

882+
// Obtain the view data
883+
$viewData = null;
884+
885+
// If setData() is currently being called, there is no need to call
886+
// mapDataToForms() here, as mapDataToForms() is called at the end
887+
// of setData() anyway. Not doing this check leads to an endless
888+
// recursion when initializing the form lazily and an event listener
889+
// (such as ResizeFormListener) adds fields depending on the data:
890+
//
891+
// * setData() is called, the form is not initialized yet
892+
// * add() is called by the listener (setData() is not complete, so
893+
// the form is still not initialized)
894+
// * getViewData() is called
895+
// * setData() is called since the form is not initialized yet
896+
// * ... endless recursion ...
897+
if (!$this->lockSetData) {
898+
$viewData = $this->getViewData();
899+
}
900+
836901
$this->children[$child->getName()] = $child;
837902

838903
$child->setParent($this);
839904

840-
$this->config->getDataMapper()->mapDataToForms($this->getViewData(), array($child));
905+
if (!$this->lockSetData) {
906+
$this->config->getDataMapper()->mapDataToForms($viewData, array($child));
907+
}
841908

842909
return $this;
843910
}

‎src/Symfony/Component/Form/Tests/CompoundFormTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
use Symfony\Component\EventDispatcher\EventDispatcher;
2121
use Symfony\Component\Form\Tests\Fixtures\FixedDataTransformer;
2222

23-
class FormTest extends AbstractFormTest
23+
class CompoundFormTest extends AbstractFormTest
2424
{
2525
public function testValidIfAllChildrenAreValid()
2626
{

‎src/Symfony/Component/Form/Tests/SimpleFormTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\Component\Form\Tests;
1313

1414
use Symfony\Component\Form\Form;
15+
use Symfony\Component\Form\FormEvent;
16+
use Symfony\Component\Form\FormEvents;
1517
use Symfony\Component\Form\Util\PropertyPath;
1618
use Symfony\Component\Form\FormConfig;
1719
use Symfony\Component\Form\FormView;
@@ -732,6 +734,21 @@ public function testViewDataMustBeObjectIfDataClassIsSet()
732734
$form->setData('foo');
733735
}
734736

737+
/**
738+
* @expectedException Symfony\Component\Form\Exception\FormException
739+
*/
740+
public function testSetDataCannotInvokeItself()
741+
{
742+
// Cycle detection to prevent endless loops
743+
$config = new FormConfig('name', 'stdClass', $this->dispatcher);
744+
$config->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) {
745+
$event->getForm()->setData('bar');
746+
});
747+
$form = new Form($config);
748+
749+
$form->setData('foo');
750+
}
751+
735752
protected function createForm()
736753
{
737754
return $this->getBuilder()->getForm();

0 commit comments

Comments
 (0)
0