diff --git a/UPGRADE-2.3.md b/UPGRADE-2.3.md index f58f837b64ff2..1f43bf1aab41c 100644 --- a/UPGRADE-2.3.md +++ b/UPGRADE-2.3.md @@ -1,4 +1,4 @@ -UPGRADE FROM 2.2 to 2.3 +UPGRADE FROM 2.2 to 2.3 ======================= ### Form @@ -35,6 +35,79 @@ UPGRADE FROM 2.2 to 2.3 "validation_groups" => false "validation_groups" => array() ``` + * The array type hint from DataMapperInterface was removed. You should adapt + implementations of that interface accordingly. + + Before: + + ``` + use Symfony\Component\Form\DataMapperInterface; + + class MyDataMapper + { + public function mapFormsToData(array $forms, $data) + { + // ... + } + + public function mapDataToForms($data, array $forms) + { + // ... + } + } + ``` + + After: + + ``` + use Symfony\Component\Form\DataMapperInterface; + + class MyDataMapper + { + public function mapFormsToData($forms, $data) + { + // ... + } + + public function mapDataToForms($data, $forms) + { + // ... + } + } + ``` + + Instead of an array, the methods here are now passed a + RecursiveIteratorIterator containing an InheritDataAwareIterator by default, + so you don't need to handle forms inheriting their parent data (former + "virtual forms") in the data mapper anymore. + + Before: + + ``` + use Symfony\Component\Form\Util\VirtualFormAwareIterator; + + public function mapFormsToData(array $forms, $data) + { + $iterator = new \RecursiveIteratorIterator( + new VirtualFormAwareIterator($forms) + ); + + foreach ($iterator as $form) { + // ... + } + } + ``` + + After: + + ``` + public function mapFormsToData($forms, $data) + { + foreach ($forms as $form) { + // ... + } + } + ``` ### PropertyAccess diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index 42b0542845620..580c8e8b42106 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -57,6 +57,42 @@ UPGRADE FROM 2.x to 3.0 } ``` + * The option "virtual" was renamed to "inherit_data". + + Before: + + ``` + $builder->add('address', 'form', array( + 'virtual' => true, + )); + ``` + + After: + + ``` + $builder->add('address', 'form', array( + 'inherit_data' => true, + )); + ``` + + * The class VirtualFormAwareIterator was renamed to InheritDataAwareIterator. + + Before: + + ``` + use Symfony\Component\Form\Util\VirtualFormAwareIterator; + + $iterator = new VirtualFormAwareIterator($forms); + ``` + + After: + + ``` + use Symfony\Component\Form\Util\InheritDataAwareIterator; + + $iterator = new InheritDataAwareIterator($forms); + ``` + ### FrameworkBundle * The `enctype` method of the `form` helper was removed. You should use the diff --git a/src/Symfony/Component/Form/ButtonBuilder.php b/src/Symfony/Component/Form/ButtonBuilder.php index fb77ef00ff12f..66ce61ab53ff6 100644 --- a/src/Symfony/Component/Form/ButtonBuilder.php +++ b/src/Symfony/Component/Form/ButtonBuilder.php @@ -492,6 +492,18 @@ public function setFormProcessor(FormProcessorInterface $formProcessor) throw new \BadMethodCallException('Buttons do not support form processors.'); } + /** + * Unsupported method. + * + * @param Boolean $inheritData + * + * @throws \BadMethodCallException + */ + public function setInheritData($inheritData) + { + throw new \BadMethodCallException('Buttons do not support data inheritance.'); + } + /** * Builds and returns the button configuration. * @@ -759,6 +771,16 @@ public function getFormProcessor() return null; } + /** + * Unsupported method. + * + * @return null Always returns null. + */ + public function getInheritData() + { + return null; + } + /** * Returns all options passed during the construction of the button. * diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index 40976fa8d06c4..25fdc6d8f049f 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -9,6 +9,10 @@ CHANGELOG * added FormProcessorInterface and FormInterface::process() * deprecated passing a Request instance to FormInterface::bind() * added options "method" and "action" to FormType + * deprecated option "virtual" in favor "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 a574cbc8dec3c..6e0316829152d 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 0000000000000..0af48a4a21505 --- /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 f691ecca21502..d8bd9c715bb85 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 @@ public function __construct(PropertyAccessorInterface $propertyAccessor = null) /** * {@inheritdoc} */ - public function mapDataToForms($data, array $forms) + public function mapDataToForms($data, $forms) { if (null === $data || array() === $data) { return; @@ -52,11 +51,7 @@ public function mapDataToForms($data, array $forms) throw new UnexpectedTypeException($data, 'object, array or empty'); } - $iterator = new VirtualFormAwareIterator($forms); - $iterator = new \RecursiveIteratorIterator($iterator); - - foreach ($iterator as $form) { - /* @var FormInterface $form */ + foreach ($forms as $form) { $propertyPath = $form->getPropertyPath(); $config = $form->getConfig(); @@ -69,7 +64,7 @@ public function mapDataToForms($data, array $forms) /** * {@inheritdoc} */ - public function mapFormsToData(array $forms, &$data) + public function mapFormsToData($forms, &$data) { if (null === $data) { return; @@ -79,11 +74,7 @@ public function mapFormsToData(array $forms, &$data) throw new UnexpectedTypeException($data, 'object, array or empty'); } - $iterator = new VirtualFormAwareIterator($forms); - $iterator = new \RecursiveIteratorIterator($iterator); - - foreach ($iterator as $form) { - /* @var FormInterface $form */ + foreach ($forms as $form) { $propertyPath = $form->getPropertyPath(); $config = $form->getConfig(); diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php index e51dba870e1fa..473b009ea3741 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php @@ -48,7 +48,7 @@ public function buildForm(FormBuilderInterface $builder, array $options) ->setPropertyPath($options['property_path']) ->setMapped($options['mapped']) ->setByReference($options['by_reference']) - ->setVirtual($options['virtual']) + ->setInheritData($options['inherit_data']) ->setCompound($options['compound']) ->setData(isset($options['data']) ? $options['data'] : null) ->setDataLocked(isset($options['data'])) @@ -95,8 +95,8 @@ public function buildView(FormView $view, FormInterface $form, array $options) 'size' => null, 'label_attr' => $options['label_attr'], 'compound' => $form->getConfig()->getCompound(), - 'method' => $form->getConfig()->getMethod(), - 'action' => $form->getConfig()->getAction(), + 'method' => $form->getConfig()->getMethod(), + 'action' => $form->getConfig()->getAction(), )); } @@ -150,6 +150,18 @@ public function setDefaultOptions(OptionsResolverInterface $resolver) return $options['compound']; }; + // BC with old "virtual" option + $inheritData = function (Options $options) { + if (null !== $options['virtual']) { + // Uncomment this as soon as the deprecation note should be shown + // trigger_error('The form option "virtual" is deprecated since version 2.3 and will be removed in 3.0. Use "inherit_data" instead.', E_USER_DEPRECATED); + + return $options['virtual']; + } + + return false; + }; + // If data is given, the form is locked to that data // (independent of its value) $resolver->setOptional(array( @@ -169,7 +181,8 @@ public function setDefaultOptions(OptionsResolverInterface $resolver) 'by_reference' => true, 'error_bubbling' => $errorBubbling, 'label_attr' => array(), - 'virtual' => false, + 'virtual' => null, + 'inherit_data' => $inheritData, 'compound' => true, 'method' => 'POST', // According to RFC 2396 (http://www.ietf.org/rfc/rfc2396.txt) diff --git a/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php b/src/Symfony/Component/Form/Extension/Validator/ViolationMapper/ViolationMapper.php index 1c1e64f94f8a7..8a7636c7e88ac 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; @@ -88,8 +88,8 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form } // This case happens if an error happened in the data under a - // virtual form that does not match any of the children of - // the virtual form. + // form inheriting its parent data that does not match any of the + // children of that form. if (null !== $violationPath && !$match) { // If we could not map the error to anything more specific // than the root element, map it to the innermost directly @@ -100,6 +100,9 @@ public function mapViolation(ConstraintViolation $violation, FormInterface $form $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 @@ -162,9 +165,9 @@ private function matchChild(FormInterface $form, PropertyPathIteratorInterface $ } } - // Ignore virtual forms when iterating the children + // 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 @@ -253,8 +256,8 @@ private function reconstructPath(ViolationPath $violationPath, FormInterface $or // Process child form $scope = $scope->get($it->current()); - if ($scope->getConfig()->getVirtual()) { - // Form is virtual + if ($scope->getConfig()->getInheritData()) { + // Form inherits its parent data // Cut the piece out of the property path and proceed $propertyPathBuilder->remove($i); } elseif (!$scope->getConfig()->getMapped()) { diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 3b95817be0d58..db884cf6f54eb 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 @@ public function __construct(FormConfigInterface $config) 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 @@ public function getPropertyPath() 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 @@ public function setData($modelData) // 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 @@ public function setData($modelData) } 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 @@ public function setData($modelData) $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 @@ public function setData($modelData) */ 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 @@ public function getData() */ 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 @@ public function getNormData() */ 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 @@ public function bind($submittedData) 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 @@ public function bind($submittedData) // 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 @@ public function bind($submittedData) $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 @@ public function bind($submittedData) } $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) { + // 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); + } - // 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; - $modelData = null; - $normData = null; + try { + // Normalize data to unified representation + $normData = $this->viewToNorm($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(); + } - // 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; } - // 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; - $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 @@ public function add($child, $type = null, array $options = array()) // * 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 @@ public function add($child, $type = null, array $options = array()) $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/FormConfigBuilder.php b/src/Symfony/Component/Form/FormConfigBuilder.php index 694b94555e90d..0927c729212bf 100644 --- a/src/Symfony/Component/Form/FormConfigBuilder.php +++ b/src/Symfony/Component/Form/FormConfigBuilder.php @@ -80,7 +80,7 @@ class FormConfigBuilder implements FormConfigBuilderInterface /** * @var Boolean */ - private $virtual = false; + private $inheritData = false; /** * @var Boolean @@ -341,9 +341,25 @@ public function getByReference() /** * {@inheritdoc} */ + public function getInheritData() + { + return $this->inheritData; + } + + /** + * Alias of {@link getInheritData()}. + * + * @return FormConfigBuilder The configuration object. + * + * @deprecated Deprecated since version 2.3, to be removed in 3.0. Use + * {@link getInheritData()} instead. + */ public function getVirtual() { - return $this->virtual; + // Uncomment this as soon as the deprecation note should be shown + // trigger_error('getVirtual() is deprecated since version 2.3 and will be removed in 3.0. Use getInheritData() instead.', E_USER_DEPRECATED); + + return $this->getInheritData(); } /** @@ -676,17 +692,35 @@ public function setByReference($byReference) /** * {@inheritdoc} */ - public function setVirtual($virtual) + public function setInheritData($inheritData) { if ($this->locked) { throw new BadMethodCallException('FormConfigBuilder methods cannot be accessed anymore once the builder is turned into a FormConfigInterface instance.'); } - $this->virtual = $virtual; + $this->inheritData = $inheritData; return $this; } + /** + * Alias of {@link setInheritData()}. + * + * @param Boolean $inheritData Whether the form should inherit its parent's data. + * + * @return FormConfigBuilder The configuration object. + * + * @deprecated Deprecated since version 2.3, to be removed in 3.0. Use + * {@link setInheritData()} instead. + */ + public function setVirtual($inheritData) + { + // Uncomment this as soon as the deprecation note should be shown + // trigger_error('setVirtual() is deprecated since version 2.3 and will be removed in 3.0. Use setInheritData() instead.', E_USER_DEPRECATED); + + $this->setInheritData($inheritData); + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Form/FormConfigBuilderInterface.php b/src/Symfony/Component/Form/FormConfigBuilderInterface.php index 34b217e623779..a35be27db2759 100644 --- a/src/Symfony/Component/Form/FormConfigBuilderInterface.php +++ b/src/Symfony/Component/Form/FormConfigBuilderInterface.php @@ -180,13 +180,13 @@ public function setMapped($mapped); public function setByReference($byReference); /** - * Sets whether the form should be virtual. + * Sets whether the form should read and write the data of its parent. * - * @param Boolean $virtual Whether the form should be virtual. + * @param Boolean $inheritData Whether the form should inherit its parent's data. * * @return self The configuration object. */ - public function setVirtual($virtual); + public function setInheritData($inheritData); /** * Sets whether the form should be compound. diff --git a/src/Symfony/Component/Form/FormConfigInterface.php b/src/Symfony/Component/Form/FormConfigInterface.php index e00ce27f43d9d..3aa00d4dbdd2e 100644 --- a/src/Symfony/Component/Form/FormConfigInterface.php +++ b/src/Symfony/Component/Form/FormConfigInterface.php @@ -55,15 +55,11 @@ public function getMapped(); public function getByReference(); /** - * Returns whether the form should be virtual. + * Returns whether the form should read and write the data of its parent. * - * When mapping data to the children of a form, the data mapper - * should ignore virtual forms and map to the children of the - * virtual form instead. - * - * @return Boolean Whether the form is virtual. + * @return Boolean Whether the form should inherit its parent's data. */ - public function getVirtual(); + public function getInheritData(); /** * Returns whether the form is compound. diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php index 1d52075d6cfaa..fc3b5a6d6d12f 100644 --- a/src/Symfony/Component/Form/Tests/CompoundFormTest.php +++ b/src/Symfony/Component/Form/Tests/CompoundFormTest.php @@ -264,6 +264,7 @@ public function testIterator() public function testAddMapsViewDataToForm() { + $test = $this; $mapper = $this->getDataMapper(); $form = $this->getBuilder() ->setCompound(true) @@ -278,13 +279,34 @@ public function testAddMapsViewDataToForm() $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 @@ public function testSetDataMapsViewDataToChildren() $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 @@ public function testBindMapsBoundChildrenOntoExistingViewData() $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 @@ public function testBindMapsBoundChildrenOntoExistingViewData() )); } + 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 @@ public function testBindRestoresViewDataIfCompoundAndEmpty() public function testBindMapsBoundChildrenOntoEmptyData() { + $test = $this; $mapper = $this->getDataMapper(); $object = new \stdClass(); $form = $this->getBuilder() @@ -369,7 +423,11 @@ public function testBindMapsBoundChildrenOntoEmptyData() $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 8af2fd5f07f15..ee2e3351ce9dd 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 @@ public function testMapDataToFormsIgnoresEmptyData() $this->assertNull($form->getData()); } - public function testMapDataToFormsSkipsVirtualForms() - { - $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->setVirtual(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 @@ public function testMapFormsToDataIgnoresDisabled() $this->mapper->mapFormsToData(array($form), $car); } - - public function testMapFormsToDataSkipsVirtualForms() - { - $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->setVirtual(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 83a5db6e1040b..cbe1acbe9c9df 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/ViolationMapper/ViolationMapperTest.php @@ -71,13 +71,13 @@ protected function setUp() $this->params = array('foo' => 'bar'); } - protected function getForm($name = 'name', $propertyPath = null, $dataClass = null, $errorMapping = array(), $virtual = false, $synchronized = true) + protected function getForm($name = 'name', $propertyPath = null, $dataClass = null, $errorMapping = array(), $inheritData = false, $synchronized = true) { $config = new FormConfigBuilder($name, $dataClass, $this->dispatcher, array( 'error_mapping' => $errorMapping, )); $config->setMapped(true); - $config->setVirtual($virtual); + $config->setInheritData($inheritData); $config->setPropertyPath($propertyPath); $config->setCompound(true); $config->setDataMapper($this->getDataMapper()); @@ -118,7 +118,7 @@ protected function getFormError() return new FormError($this->message, $this->messageTemplate, $this->params); } - public function testMapToVirtualFormIfDataDoesNotMatch() + public function testMapToFormInheritingParentDataIfDataDoesNotMatch() { $violation = $this->getConstraintViolation('children[address].data.foo'); $parent = $this->getForm('parent'); @@ -183,28 +183,6 @@ public function testAbortMappingIfNotSynchronized() $this->assertCount(0, $grandChild->getErrors(), $grandChild->getName().' should not have an error, but has one'); } - public function testAbortVirtualFormMappingIfNotSynchronized() - { - $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 @@ public function testCustomFormErrorMapping($target, $mapFrom, $mapTo, $errorName } } - public function provideVirtualFormErrorTests() + public function provideErrorTestsForFormInheritingParentData() { return array( // mapping target, child name, its property path, grand child name, its property path, violation path @@ -1472,9 +1450,9 @@ public function provideVirtualFormErrorTests() } /** - * @dataProvider provideVirtualFormErrorTests + * @dataProvider provideErrorTestsForFormInheritingParentData */ - public function testVirtualFormErrorMapping($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 9493b85ce1b12..65f3ee2949b93 100644 --- a/src/Symfony/Component/Form/Tests/SimpleFormTest.php +++ b/src/Symfony/Component/Form/Tests/SimpleFormTest.php @@ -763,6 +763,42 @@ public function testGetPropertyPathDefaultsToIndexedNameIfParentDataClassIsNull( $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 @@ public function testViewDataMustBeObjectIfDataClassIsSet() } /** - * @expectedException \Symfony\Component\Form\Exception\Exception + * @expectedException \Symfony\Component\Form\Exception\RuntimeException */ public function testSetDataCannotInvokeItself() { @@ -857,6 +893,103 @@ public function testProcessForwardsToFormProcessor() $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 0000000000000..5c2c5fadccff7 --- /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 c1322bb35d939..24fdc8bb9fcec 100644 --- a/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php +++ b/src/Symfony/Component/Form/Util/VirtualFormAwareIterator.php @@ -12,22 +12,39 @@ namespace Symfony\Component\Form\Util; /** - * Iterator that traverses fields of a field group + * Iterator that returns only forms from a form tree that do not inherit their + * parent data. * - * If the iterator encounters a virtual field group, it enters the field - * group and traverses its children as well. + * If the iterator encounters a form that inherits its parent data, it enters + * the form and traverses its children as well. * * @author Bernhard Schussek + * + * @deprecated Deprecated since version 2.3, to be removed in 3.0. 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) + { + // Uncomment this as soon as the deprecation note should be shown + // trigger_error('VirtualFormAwareIterator is deprecated since version 2.3 and will be removed in 3.0. 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() { - return $this->current()->getConfig()->getVirtual(); + return $this->current()->getConfig()->getInheritData(); } }