diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index b7b6b08245..9fdeaffcdd 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -10,6 +10,9 @@ CHANGELOG * deprecated passing a Request instance to FormInterface::bind() * added options "method" and "action" to FormType * deprecated option "virtual", renamed it to "inherit_data" + * deprecated VirtualFormAwareIterator in favor of InheritDataAwareIterator + * [BC BREAK] removed the "array" type hint from DataMapperInterface + * improved forms inheriting their parent data to actually return that data from getData(), getNormData() and getViewData() 2.2.0 ----- diff --git a/src/Symfony/Component/Form/DataMapperInterface.php b/src/Symfony/Component/Form/DataMapperInterface.php index a574cbc8de..6e03168291 100644 --- a/src/Symfony/Component/Form/DataMapperInterface.php +++ b/src/Symfony/Component/Form/DataMapperInterface.php @@ -19,20 +19,20 @@ interface DataMapperInterface /** * Maps properties of some data to a list of forms. * - * @param mixed $data Structured data. - * @param array $forms A list of {@link FormInterface} instances. + * @param mixed $data Structured data. + * @param FormInterface[] $forms A list of {@link FormInterface} instances. * * @throws Exception\UnexpectedTypeException if the type of the data parameter is not supported. */ - public function mapDataToForms($data, array $forms); + public function mapDataToForms($data, $forms); /** * Maps the data of a list of forms into the properties of some data. * - * @param array $forms A list of {@link FormInterface} instances. - * @param mixed $data Structured data. + * @param FormInterface[] $forms A list of {@link FormInterface} instances. + * @param mixed $data Structured data. * * @throws Exception\UnexpectedTypeException if the type of the data parameter is not supported. */ - public function mapFormsToData(array $forms, &$data); + public function mapFormsToData($forms, &$data); } diff --git a/src/Symfony/Component/Form/Exception/RuntimeException.php b/src/Symfony/Component/Form/Exception/RuntimeException.php new file mode 100644 index 0000000000..0af48a4a21 --- /dev/null +++ b/src/Symfony/Component/Form/Exception/RuntimeException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Exception; + +/** + * Base RuntimeException for the Form component. + * + * @author Bernhard Schussek + */ +class RuntimeException extends \RuntimeException implements ExceptionInterface +{ +} diff --git a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php index cb59baa075..d8bd9c715b 100644 --- a/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php +++ b/src/Symfony/Component/Form/Extension/Core/DataMapper/PropertyPathMapper.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Form\Extension\Core\DataMapper; use Symfony\Component\Form\DataMapperInterface; -use Symfony\Component\Form\Util\VirtualFormAwareIterator; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\PropertyAccess\PropertyAccess; use Symfony\Component\PropertyAccess\PropertyAccessorInterface; @@ -42,7 +41,7 @@ class PropertyPathMapper implements DataMapperInterface /** * {@inheritdoc} */ - public function mapDataToForms($data, array $forms) + public function mapDataToForms($data, $forms) { if (null === $data || array() === $data) { return; @@ -52,11 +51,7 @@ class PropertyPathMapper implements DataMapperInterface throw new UnexpectedTypeException($data, 'object, array or empty'); } - $iterator = new VirtualFormAwareIterator($forms); - $iterator = new \RecursiveIteratorIterator($iterator); - - foreach ($iterator as $form) { - /* @var \Symfony\Component\Form\FormInterface $form */ + foreach ($forms as $form) { $propertyPath = $form->getPropertyPath(); $config = $form->getConfig(); @@ -69,7 +64,7 @@ class PropertyPathMapper implements DataMapperInterface /** * {@inheritdoc} */ - public function mapFormsToData(array $forms, &$data) + public function mapFormsToData($forms, &$data) { if (null === $data) { return; @@ -79,11 +74,7 @@ class PropertyPathMapper implements DataMapperInterface throw new UnexpectedTypeException($data, 'object, array or empty'); } - $iterator = new VirtualFormAwareIterator($forms); - $iterator = new \RecursiveIteratorIterator($iterator); - - foreach ($iterator as $form) { - /* @var \Symfony\Component\Form\FormInterface $form */ + foreach ($forms as $form) { $propertyPath = $form->getPropertyPath(); $config = $form->getConfig(); diff --git a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php index 4eb9126b6b..8a7636c7e8 100644 --- a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php +++ b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php @@ -12,7 +12,7 @@ namespace Symfony\Component\Form\Extension\Validator\ViolationMapper; use Symfony\Component\Form\FormInterface; -use Symfony\Component\Form\Util\VirtualFormAwareIterator; +use Symfony\Component\Form\Util\InheritDataAwareIterator; use Symfony\Component\PropertyAccess\PropertyPathIterator; use Symfony\Component\PropertyAccess\PropertyPathBuilder; use Symfony\Component\PropertyAccess\PropertyPathIteratorInterface; @@ -100,6 +100,9 @@ class ViolationMapper implements ViolationMapperInterface $scope = $form; $it = new ViolationPathIterator($violationPath); + // Note: acceptsErrors() will always return true for forms inheriting + // their parent data, because these forms can never be non-synchronized + // (they don't do any data transformation on their own) while ($this->acceptsErrors($scope) && $it->valid() && $it->mapsForm()) { if (!$scope->has($it->current())) { // Break if we find a reference to a non-existing child @@ -164,7 +167,7 @@ class ViolationMapper implements ViolationMapperInterface // Skip forms inheriting their parent data when iterating the children $childIterator = new \RecursiveIteratorIterator( - new VirtualFormAwareIterator($form->all()) + new InheritDataAwareIterator($form->all()) ); // Make the path longer until we find a matching child diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 3b95817be0..db884cf6f5 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -11,11 +11,14 @@ namespace Symfony\Component\Form; +use Symfony\Component\Form\Exception\BadMethodCallException; use Symfony\Component\Form\Exception\Exception; +use Symfony\Component\Form\Exception\RuntimeException; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\Form\Exception\AlreadyBoundException; use Symfony\Component\Form\Exception\TransformationFailedException; use Symfony\Component\Form\Util\FormUtil; +use Symfony\Component\Form\Util\InheritDataAwareIterator; use Symfony\Component\PropertyAccess\PropertyPath; /** @@ -130,7 +133,7 @@ class Form implements \IteratorAggregate, FormInterface * * @var Boolean */ - private $initialized = false; + private $defaultDataSet = false; /** * Whether setData() is currently being called. @@ -143,7 +146,7 @@ class Form implements \IteratorAggregate, FormInterface * * @param FormConfigInterface $config The form configuration. * - * @throws FormException if a data mapper is not provided for a compound form + * @throws Exception if a data mapper is not provided for a compound form */ public function __construct(FormConfigInterface $config) { @@ -154,6 +157,12 @@ class Form implements \IteratorAggregate, FormInterface throw new Exception('Compound forms need a data mapper'); } + // If the form inherits the data from its parent, it is not necessary + // to call setData() with the default data. + if ($config->getInheritData()) { + $this->defaultDataSet = true; + } + $this->config = $config; } @@ -193,7 +202,13 @@ class Form implements \IteratorAggregate, FormInterface return null; } - if ($this->parent && null === $this->parent->getConfig()->getDataClass()) { + $parent = $this->parent; + + while ($parent && $parent->getConfig()->getInheritData()) { + $parent = $parent->getParent(); + } + + if ($parent && null === $parent->getConfig()->getDataClass()) { return new PropertyPath('['.$this->getName().']'); } @@ -274,8 +289,14 @@ class Form implements \IteratorAggregate, FormInterface // If the form is bound while disabled, it is set to bound, but the data is not // changed. In such cases (i.e. when the form is not initialized yet) don't // abort this method. - if ($this->bound && $this->initialized) { - throw new AlreadyBoundException('You cannot change the data of a bound form'); + if ($this->bound && $this->defaultDataSet) { + throw new AlreadyBoundException('You cannot change the data of a bound form.'); + } + + // If the form inherits its parent's data, disallow data setting to + // prevent merge conflicts + if ($this->config->getInheritData()) { + throw new RuntimeException('You cannot change the data of a form inheriting its parent data.'); } // Don't allow modifications of the configured data if the data is locked @@ -288,7 +309,7 @@ class Form implements \IteratorAggregate, FormInterface } if ($this->lockSetData) { - throw new Exception('A cycle was detected. Listeners to the PRE_SET_DATA event must not call setData(). You should call setData() on the FormEvent object instead.'); + throw new RuntimeException('A cycle was detected. Listeners to the PRE_SET_DATA event must not call setData(). You should call setData() on the FormEvent object instead.'); } $this->lockSetData = true; @@ -342,14 +363,16 @@ class Form implements \IteratorAggregate, FormInterface $this->modelData = $modelData; $this->normData = $normData; $this->viewData = $viewData; - $this->initialized = true; + $this->defaultDataSet = true; $this->lockSetData = false; // It is not necessary to invoke this method if the form doesn't have children, // even if the form is compound. if (count($this->children) > 0) { // Update child forms from the data - $this->config->getDataMapper()->mapDataToForms($viewData, $this->children); + $childrenIterator = new InheritDataAwareIterator($this->children); + $childrenIterator = new \RecursiveIteratorIterator($childrenIterator); + $this->config->getDataMapper()->mapDataToForms($viewData, $childrenIterator); } if ($dispatcher->hasListeners(FormEvents::POST_SET_DATA)) { @@ -365,7 +388,15 @@ class Form implements \IteratorAggregate, FormInterface */ public function getData() { - if (!$this->initialized) { + if ($this->config->getInheritData()) { + if (!$this->parent) { + throw new RuntimeException('The form is configured to inherit its parent\'s data, but does not have a parent.'); + } + + return $this->parent->getData(); + } + + if (!$this->defaultDataSet) { $this->setData($this->config->getData()); } @@ -377,7 +408,15 @@ class Form implements \IteratorAggregate, FormInterface */ public function getNormData() { - if (!$this->initialized) { + if ($this->config->getInheritData()) { + if (!$this->parent) { + throw new RuntimeException('The form is configured to inherit its parent\'s data, but does not have a parent.'); + } + + return $this->parent->getNormData(); + } + + if (!$this->defaultDataSet) { $this->setData($this->config->getData()); } @@ -389,7 +428,15 @@ class Form implements \IteratorAggregate, FormInterface */ public function getViewData() { - if (!$this->initialized) { + if ($this->config->getInheritData()) { + if (!$this->parent) { + throw new RuntimeException('The form is configured to inherit its parent\'s data, but does not have a parent.'); + } + + return $this->parent->getViewData(); + } + + if (!$this->defaultDataSet) { $this->setData($this->config->getData()); } @@ -423,6 +470,11 @@ class Form implements \IteratorAggregate, FormInterface throw new AlreadyBoundException('A form can only be bound once'); } + // Initialize errors in the very beginning so that we don't lose any + // errors added during listeners + $this->errors = array(); + + // Obviously, a disabled form should not change its data upon binding. if ($this->isDisabled()) { $this->bound = true; @@ -432,7 +484,7 @@ class Form implements \IteratorAggregate, FormInterface // The data must be initialized if it was not initialized yet. // This is necessary to guarantee that the *_SET_DATA listeners // are always invoked before bind() takes place. - if (!$this->initialized) { + if (!$this->defaultDataSet) { $this->setData($this->config->getData()); } @@ -444,10 +496,6 @@ class Form implements \IteratorAggregate, FormInterface $submittedData = (string) $submittedData; } - // Initialize errors in the very beginning so that we don't lose any - // errors added during listeners - $this->errors = array(); - $dispatcher = $this->config->getEventDispatcher(); // Hook to change content of the data bound by the browser @@ -472,63 +520,83 @@ class Form implements \IteratorAggregate, FormInterface } $this->extraData = $submittedData; + } + // Forms that inherit their parents' data also are not processed, + // because then it would be too difficult to merge the changes in + // the child and the parent form. Instead, the parent form also takes + // changes in the grandchildren (i.e. children of the form that inherits + // its parent's data) into account. + // (see InheritDataAwareIterator below) + if ($this->config->getInheritData()) { + $this->bound = true; + + // When POST_BIND is reached, the data is not yet updated, so pass + // NULL to prevent hard-to-debug bugs. + $dataForPostBind = null; + } else { // If the form is compound, the default data in view format // is reused. The data of the children is merged into this // default data using the data mapper. - $viewData = $this->viewData; - } else { // If the form is not compound, the submitted data is also the data in view format. - $viewData = $submittedData; - } + $viewData = $this->config->getCompound() ? $this->viewData : $submittedData; - if (FormUtil::isEmpty($viewData)) { - $emptyData = $this->config->getEmptyData(); + if (FormUtil::isEmpty($viewData)) { + $emptyData = $this->config->getEmptyData(); - if ($emptyData instanceof \Closure) { - /* @var \Closure $emptyData */ - $emptyData = $emptyData($this, $viewData); + if ($emptyData instanceof \Closure) { + /* @var \Closure $emptyData */ + $emptyData = $emptyData($this, $viewData); + } + + $viewData = $emptyData; } - $viewData = $emptyData; - } - - // Merge form data from children into existing view data - // It is not necessary to invoke this method if the form has no children, - // even if it is compound. - if (count($this->children) > 0) { - $this->config->getDataMapper()->mapFormsToData($this->children, $viewData); - } - - $modelData = null; - $normData = null; - - try { - // Normalize data to unified representation - $normData = $this->viewToNorm($viewData); - - // Hook to change content of the data into the normalized - // representation - if ($dispatcher->hasListeners(FormEvents::BIND)) { - $event = new FormEvent($this, $normData); - $dispatcher->dispatch(FormEvents::BIND, $event); - $normData = $event->getData(); + // Merge form data from children into existing view data + // It is not necessary to invoke this method if the form has no children, + // even if it is compound. + if (count($this->children) > 0) { + // Use InheritDataAwareIterator to process children of + // descendants that inherit this form's data. + // These descendants will not be bound normally (see the check + // for $this->config->getInheritData() above) + $childrenIterator = new InheritDataAwareIterator($this->children); + $childrenIterator = new \RecursiveIteratorIterator($childrenIterator); + $this->config->getDataMapper()->mapFormsToData($childrenIterator, $viewData); } - // Synchronize representations - must not change the content! - $modelData = $this->normToModel($normData); - $viewData = $this->normToView($normData); - } catch (TransformationFailedException $e) { - $this->synchronized = false; - } + $modelData = null; + $normData = null; - $this->bound = true; - $this->modelData = $modelData; - $this->normData = $normData; - $this->viewData = $viewData; + try { + // Normalize data to unified representation + $normData = $this->viewToNorm($viewData); + + // Hook to change content of the data into the normalized + // representation + if ($dispatcher->hasListeners(FormEvents::BIND)) { + $event = new FormEvent($this, $normData); + $dispatcher->dispatch(FormEvents::BIND, $event); + $normData = $event->getData(); + } + + // Synchronize representations - must not change the content! + $modelData = $this->normToModel($normData); + $viewData = $this->normToView($normData); + } catch (TransformationFailedException $e) { + $this->synchronized = false; + } + + $this->bound = true; + $this->modelData = $modelData; + $this->normData = $normData; + $this->viewData = $viewData; + + $dataForPostBind = $viewData; + } if ($dispatcher->hasListeners(FormEvents::POST_BIND)) { - $event = new FormEvent($this, $viewData); + $event = new FormEvent($this, $dataForPostBind); $dispatcher->dispatch(FormEvents::POST_BIND, $event); } @@ -679,7 +747,7 @@ class Form implements \IteratorAggregate, FormInterface // * getViewData() is called // * setData() is called since the form is not initialized yet // * ... endless recursion ... - if (!$this->lockSetData) { + if (!$this->lockSetData && !$this->config->getInheritData()) { $viewData = $this->getViewData(); } @@ -703,8 +771,10 @@ class Form implements \IteratorAggregate, FormInterface $child->setParent($this); - if (!$this->lockSetData) { - $this->config->getDataMapper()->mapDataToForms($viewData, array($child)); + if (!$this->lockSetData && !$this->config->getInheritData()) { + $childrenIterator = new InheritDataAwareIterator(array($child)); + $childrenIterator = new \RecursiveIteratorIterator($childrenIterator); + $this->config->getDataMapper()->mapDataToForms($viewData, $childrenIterator); } return $this; diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php index 1d52075d6c..fc3b5a6d6d 100644 --- a/src/Symfony/Component/Form/Tests/CompoundFormTest.php +++ b/src/Symfony/Component/Form/Tests/CompoundFormTest.php @@ -264,6 +264,7 @@ class CompoundFormTest extends AbstractFormTest public function testAddMapsViewDataToForm() { + $test = $this; $mapper = $this->getDataMapper(); $form = $this->getBuilder() ->setCompound(true) @@ -278,13 +279,34 @@ class CompoundFormTest extends AbstractFormTest $child = $this->getBuilder()->getForm(); $mapper->expects($this->once()) ->method('mapDataToForms') - ->with('bar', array($child)); + ->with('bar', $this->isInstanceOf('\RecursiveIteratorIterator')) + ->will($this->returnCallback(function ($data, \RecursiveIteratorIterator $iterator) use ($child, $test) { + $test->assertInstanceOf('Symfony\Component\Form\Util\InheritDataAwareIterator', $iterator->getInnerIterator()); + $test->assertSame(array($child), iterator_to_array($iterator)); + })); + + $form->add($child); + } + + public function testAddDoesNotMapViewDataToFormIfInheritData() + { + $mapper = $this->getDataMapper(); + $form = $this->getBuilder() + ->setCompound(true) + ->setDataMapper($mapper) + ->setInheritData(true) + ->getForm(); + + $child = $this->getBuilder()->getForm(); + $mapper->expects($this->never()) + ->method('mapDataToForms'); $form->add($child); } public function testSetDataMapsViewDataToChildren() { + $test = $this; $mapper = $this->getDataMapper(); $form = $this->getBuilder() ->setCompound(true) @@ -300,7 +322,11 @@ class CompoundFormTest extends AbstractFormTest $mapper->expects($this->once()) ->method('mapDataToForms') - ->with('bar', array('firstName' => $child1, 'lastName' => $child2)); + ->with('bar', $this->isInstanceOf('\RecursiveIteratorIterator')) + ->will($this->returnCallback(function ($data, \RecursiveIteratorIterator $iterator) use ($child1, $child2, $test) { + $test->assertInstanceOf('Symfony\Component\Form\Util\InheritDataAwareIterator', $iterator->getInnerIterator()); + $test->assertSame(array('firstName' => $child1, 'lastName' => $child2), iterator_to_array($iterator)); + })); $form->setData('foo'); } @@ -324,10 +350,12 @@ class CompoundFormTest extends AbstractFormTest $mapper->expects($this->once()) ->method('mapFormsToData') - ->with(array('firstName' => $child1, 'lastName' => $child2), 'bar') - ->will($this->returnCallback(function ($children, $bar) use ($test) { - $test->assertEquals('Bernhard', $children['firstName']->getData()); - $test->assertEquals('Schussek', $children['lastName']->getData()); + ->with($this->isInstanceOf('\RecursiveIteratorIterator'), 'bar') + ->will($this->returnCallback(function (\RecursiveIteratorIterator $iterator) use ($child1, $child2, $test) { + $test->assertInstanceOf('Symfony\Component\Form\Util\InheritDataAwareIterator', $iterator->getInnerIterator()); + $test->assertSame(array('firstName' => $child1, 'lastName' => $child2), iterator_to_array($iterator)); + $test->assertEquals('Bernhard', $child1->getData()); + $test->assertEquals('Schussek', $child2->getData()); })); $form->bind(array( @@ -336,6 +364,31 @@ class CompoundFormTest extends AbstractFormTest )); } + public function testMapFormsToDataIsNotInvokedIfInheritData() + { + $mapper = $this->getDataMapper(); + $form = $this->getBuilder() + ->setCompound(true) + ->setDataMapper($mapper) + ->setInheritData(true) + ->addViewTransformer(new FixedDataTransformer(array( + '' => '', + 'foo' => 'bar', + ))) + ->getForm(); + + $form->add($child1 = $this->getBuilder('firstName')->setCompound(false)->getForm()); + $form->add($child2 = $this->getBuilder('lastName')->setCompound(false)->getForm()); + + $mapper->expects($this->never()) + ->method('mapFormsToData'); + + $form->bind(array( + 'firstName' => 'Bernhard', + 'lastName' => 'Schussek', + )); + } + /* * https://github.com/symfony/symfony/issues/4480 */ @@ -356,6 +409,7 @@ class CompoundFormTest extends AbstractFormTest public function testBindMapsBoundChildrenOntoEmptyData() { + $test = $this; $mapper = $this->getDataMapper(); $object = new \stdClass(); $form = $this->getBuilder() @@ -369,7 +423,11 @@ class CompoundFormTest extends AbstractFormTest $mapper->expects($this->once()) ->method('mapFormsToData') - ->with(array('name' => $child), $object); + ->with($this->isInstanceOf('\RecursiveIteratorIterator'), $object) + ->will($this->returnCallback(function (\RecursiveIteratorIterator $iterator) use ($child, $test) { + $test->assertInstanceOf('Symfony\Component\Form\Util\InheritDataAwareIterator', $iterator->getInnerIterator()); + $test->assertSame(array('name' => $child), iterator_to_array($iterator)); + })); $form->bind(array( 'name' => 'Bernhard', diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php index 32ef587862..ee2e3351ce 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/DataMapper/PropertyPathMapperTest.php @@ -182,37 +182,6 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase $this->assertNull($form->getData()); } - public function testMapDataToFormsSkipsFormsInheritingParentData() - { - $car = new \stdClass(); - $engine = new \stdClass(); - $propertyPath = $this->getPropertyPath('engine'); - - $this->propertyAccessor->expects($this->once()) - ->method('getValue') - ->with($car, $propertyPath) - ->will($this->returnValue($engine)); - - $config = new FormConfigBuilder('name', '\stdClass', $this->dispatcher); - $config->setByReference(true); - $config->setInheritData(true); - $config->setCompound(true); - $config->setDataMapper($this->getDataMapper()); - $form = $this->getForm($config); - - $config = new FormConfigBuilder('engine', '\stdClass', $this->dispatcher); - $config->setByReference(true); - $config->setPropertyPath($propertyPath); - $child = $this->getForm($config); - - $form->add($child); - - $this->mapper->mapDataToForms($car, array($form)); - - $this->assertNull($form->getData()); - $this->assertSame($engine, $child->getData()); - } - public function testMapFormsToDataWritesBackIfNotByReference() { $car = new \stdClass(); @@ -347,38 +316,4 @@ class PropertyPathMapperTest extends \PHPUnit_Framework_TestCase $this->mapper->mapFormsToData(array($form), $car); } - - public function testMapFormsToDataSkipsFormsInheritingParentData() - { - $car = new \stdClass(); - $engine = new \stdClass(); - $parentPath = $this->getPropertyPath('name'); - $childPath = $this->getPropertyPath('engine'); - - // getValue() and setValue() must never be invoked for $parentPath - - $this->propertyAccessor->expects($this->once()) - ->method('getValue') - ->with($car, $childPath); - $this->propertyAccessor->expects($this->once()) - ->method('setValue') - ->with($car, $childPath, $engine); - - $config = new FormConfigBuilder('name', '\stdClass', $this->dispatcher); - $config->setPropertyPath($parentPath); - $config->setInheritData(true); - $config->setCompound(true); - $config->setDataMapper($this->getDataMapper()); - $form = $this->getForm($config); - - $config = new FormConfigBuilder('engine', '\stdClass', $this->dispatcher); - $config->setByReference(true); - $config->setPropertyPath($childPath); - $config->setData($engine); - $child = $this->getForm($config); - - $form->add($child); - - $this->mapper->mapFormsToData(array($form), $car); - } } diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php index 516b62299d..cbe1acbe9c 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php @@ -183,28 +183,6 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); } - public function testAbortFormInheritingParentDataMappingIfNotSynchronized() - { - $violation = $this->getConstraintViolation('children[address].children[street].data.foo'); - $parent = $this->getForm('parent'); - $child = $this->getForm('address', 'address', null, array(), true, false); - // even though "street" is synchronized, it should not have any errors - // due to its parent not being synchronized - $grandChild = $this->getForm('street' , 'street', null, array(), true); - - $parent->add($child); - $child->add($grandChild); - - // bind to invoke the transformer and mark the form unsynchronized - $parent->bind(array()); - - $this->mapper->mapViolation($violation, $parent); - - $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'); - } - public function testAbortDotRuleMappingIfNotSynchronized() { $violation = $this->getConstraintViolation('data.address'); @@ -1446,7 +1424,7 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase } } - public function provideFormInheritingParentDataErrorTests() + public function provideErrorTestsForFormInheritingParentData() { return array( // mapping target, child name, its property path, grand child name, its property path, violation path @@ -1472,9 +1450,9 @@ class ViolationMapperTest extends \PHPUnit_Framework_TestCase } /** - * @dataProvider provideFormInheritingParentDataErrorTests + * @dataProvider provideErrorTestsForFormInheritingParentData */ - public function testFormInheritingParentDataErrorMapping($target, $childName, $childPath, $grandChildName, $grandChildPath, $violationPath) + public function testErrorMappingForFormInheritingParentData($target, $childName, $childPath, $grandChildName, $grandChildPath, $violationPath) { $violation = $this->getConstraintViolation($violationPath); $parent = $this->getForm('parent'); diff --git a/src/Symfony/Component/Form/Tests/SimpleFormTest.php b/src/Symfony/Component/Form/Tests/SimpleFormTest.php index 9493b85ce1..65f3ee2949 100644 --- a/src/Symfony/Component/Form/Tests/SimpleFormTest.php +++ b/src/Symfony/Component/Form/Tests/SimpleFormTest.php @@ -763,6 +763,42 @@ class SimpleFormTest extends AbstractFormTest $this->assertEquals(new PropertyPath('[name]'), $form->getPropertyPath()); } + public function testGetPropertyPathDefaultsToNameIfFirstParentWithoutInheritDataHasDataClass() + { + $grandParent = $this->getBuilder(null, null, 'stdClass') + ->setCompound(true) + ->setDataMapper($this->getDataMapper()) + ->getForm(); + $parent = $this->getBuilder() + ->setCompound(true) + ->setDataMapper($this->getDataMapper()) + ->setInheritData(true) + ->getForm(); + $form = $this->getBuilder('name')->getForm(); + $grandParent->add($parent); + $parent->add($form); + + $this->assertEquals(new PropertyPath('name'), $form->getPropertyPath()); + } + + public function testGetPropertyPathDefaultsToIndexedNameIfDataClassOfFirstParentWithoutInheritDataIsNull() + { + $grandParent = $this->getBuilder() + ->setCompound(true) + ->setDataMapper($this->getDataMapper()) + ->getForm(); + $parent = $this->getBuilder() + ->setCompound(true) + ->setDataMapper($this->getDataMapper()) + ->setInheritData(true) + ->getForm(); + $form = $this->getBuilder('name')->getForm(); + $grandParent->add($parent); + $parent->add($form); + + $this->assertEquals(new PropertyPath('[name]'), $form->getPropertyPath()); + } + /** * @expectedException \Symfony\Component\Form\Exception\Exception */ @@ -809,7 +845,7 @@ class SimpleFormTest extends AbstractFormTest } /** - * @expectedException \Symfony\Component\Form\Exception\Exception + * @expectedException \Symfony\Component\Form\Exception\RuntimeException */ public function testSetDataCannotInvokeItself() { @@ -857,6 +893,103 @@ class SimpleFormTest extends AbstractFormTest $this->assertSame($form, $form->process('REQUEST')); } + public function testFormInheritsParentData() + { + $child = $this->getBuilder('child') + ->setInheritData(true); + + $parent = $this->getBuilder('parent') + ->setCompound(true) + ->setDataMapper($this->getDataMapper()) + ->setData('foo') + ->addModelTransformer(new FixedDataTransformer(array( + 'foo' => 'norm[foo]', + ))) + ->addViewTransformer(new FixedDataTransformer(array( + 'norm[foo]' => 'view[foo]', + ))) + ->add($child) + ->getForm(); + + $this->assertSame('foo', $parent->get('child')->getData()); + $this->assertSame('norm[foo]', $parent->get('child')->getNormData()); + $this->assertSame('view[foo]', $parent->get('child')->getViewData()); + } + + /** + * @expectedException \Symfony\Component\Form\Exception\FormException + */ + public function testInheritDataDisallowsSetData() + { + $form = $this->getBuilder() + ->setInheritData(true) + ->getForm(); + + $form->setData('foo'); + } + + /** + * @expectedException \Symfony\Component\Form\Exception\FormException + */ + public function testGetDataRequiresParentToBeSetIfInheritData() + { + $form = $this->getBuilder() + ->setInheritData(true) + ->getForm(); + + $form->getData(); + } + + /** + * @expectedException \Symfony\Component\Form\Exception\FormException + */ + public function testGetNormDataRequiresParentToBeSetIfInheritData() + { + $form = $this->getBuilder() + ->setInheritData(true) + ->getForm(); + + $form->getNormData(); + } + + /** + * @expectedException \Symfony\Component\Form\Exception\FormException + */ + public function testGetViewDataRequiresParentToBeSetIfInheritData() + { + $form = $this->getBuilder() + ->setInheritData(true) + ->getForm(); + + $form->getViewData(); + } + + public function testPostBindDataIsNullIfInheritData() + { + $test = $this; + $form = $this->getBuilder() + ->addEventListener(FormEvents::POST_BIND, function (FormEvent $event) use ($test) { + $test->assertNull($event->getData()); + }) + ->setInheritData(true) + ->getForm(); + + $form->bind('foo'); + } + + public function testBindIsNeverFiredIfInheritData() + { + $test = $this; + $form = $this->getBuilder() + ->addEventListener(FormEvents::BIND, function (FormEvent $event) use ($test) { + $test->fail('The BIND event should not be fired'); + }) + ->setInheritData(true) + ->getForm(); + + $form->bind('foo'); + } + protected function createForm() { return $this->getBuilder()->getForm(); diff --git a/src/Symfony/Component/Form/Util/InheritDataAwareIterator.php b/src/Symfony/Component/Form/Util/InheritDataAwareIterator.php new file mode 100644 index 0000000000..5c2c5fadcc --- /dev/null +++ b/src/Symfony/Component/Form/Util/InheritDataAwareIterator.php @@ -0,0 +1,35 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Util; + +/** + * Iterator that returns only forms from a form tree that do not inherit their + * parent data. + * + * If the iterator encounters a form that inherits its parent data, it enters + * the form and traverses its children as well. + * + * @author Bernhard Schussek + */ +class InheritDataAwareIterator extends VirtualFormAwareIterator +{ + /** + * Creates a new iterator. + * + * @param \Symfony\Component\Form\FormInterface[] $forms An array + */ + public function __construct(array $forms) + { + // Skip the deprecation error + \ArrayIterator::__construct($forms); + } +} diff --git a/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php b/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php index 92e9da0d51..55c82ab550 100644 --- a/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php +++ b/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php @@ -19,12 +19,27 @@ namespace Symfony\Component\Form\Util; * the form and traverses its children as well. * * @author Bernhard Schussek + * + * @deprecated Deprecated since version 2.2, to be removed in 2.3. Use + * {@link InheritDataAwareIterator} instead. */ class VirtualFormAwareIterator extends \ArrayIterator implements \RecursiveIterator { + /** + * Creates a new iterator. + * + * @param \Symfony\Component\Form\FormInterface[] $forms An array + */ + public function __construct(array $forms) + { + trigger_error('VirtualFormAwareIterator is deprecated since version 2.2 and will be removed in 2.3. Use InheritDataAwareIterator instead.', E_USER_DEPRECATED); + + parent::__construct($forms); + } + public function getChildren() { - return new self($this->current()->all()); + return new static($this->current()->all()); } public function hasChildren()