Skip to content

Commit

Permalink
[Form] Made data mappers completely responsible for dealing with empt…
Browse files Browse the repository at this point in the history
…y values. Removed duplication of code.
  • Loading branch information
webmozart committed Jul 6, 2012
1 parent 9bf6e8b commit 5fe3f39
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 23 deletions.
Expand Up @@ -23,22 +23,24 @@ class PropertyPathMapper implements DataMapperInterface
*/
public function mapDataToForms($data, array $forms)
{
if (!empty($data) && !is_array($data) && !is_object($data)) {
if (null === $data || array() === $data) {
return;
}

if (!is_array($data) && !is_object($data)) {
throw new UnexpectedTypeException($data, 'object, array or empty');

This comment has been minimized.

Copy link
@MDrollette

MDrollette Jul 10, 2012

Contributor

What is the reason for removing the empty() check? I'm now getting this exception even though $data is an empty string.

http://s.drollette.com/1W1w441L3t2Y1h1G2E3I

This comment has been minimized.

Copy link
@webmozart

webmozart Jul 10, 2012

Author Contributor

Can you provide some reproducible code?

This comment has been minimized.

Copy link
@MDrollette

MDrollette Jul 10, 2012

Contributor

Ok, I've managed to resolve it. I'll just post the changes here incase anyone finds this random comment with a similar issue.

In one case it was from having <argument /> as the third argument to FormFactory->createNamed(), i just had to change it to null explicitly:

    <service id="omg_user.admin_user.form" factory-method="createNamed" factory-service="form.factory" class="Symfony\Component\Form\Form">
        <argument>%omg_user.admin_user.form.name%</argument>
        <argument type="service" id="omg_user.admin_user.form.type" />
        <argument />
        <argument type="collection">
            <argument key="data_class">%fos_user.model.user.class%</argument>
            <argument key="validation_groups">%omg_user.admin_user.form.validation_groups%</argument>
        </argument>
    </service>

becomes:

    <service id="omg_user.admin_user.form" factory-method="createNamed" factory-service="form.factory" class="Symfony\Component\Form\Form">
        <argument>%omg_user.admin_user.form.name%</argument>
        <argument type="service" id="omg_user.admin_user.form.type" />
        <argument>null</argument>
        <argument type="collection">
            <argument key="data_class">%fos_user.model.user.class%</argument>
            <argument key="validation_groups">%omg_user.admin_user.form.validation_groups%</argument>
        </argument>
    </service>

The other case was a DataTransformer

   $builder->addModelTransformer(
        new CallbackTransformer(
            function($value)
            {
                if (null === $value) {
                    return '';
                }

                return array('id' => $value, 'name' => $value->getName());
            },
            function($value)
            {
                if (null === $value || '' === $value) {
                    return null;
                }

                return $value['id'];
            }
        )
    );

I had to change to return null; instead of return '';

}

if (!empty($data)) {
$iterator = new VirtualFormAwareIterator($forms);
$iterator = new \RecursiveIteratorIterator($iterator);
$iterator = new VirtualFormAwareIterator($forms);
$iterator = new \RecursiveIteratorIterator($iterator);

foreach ($iterator as $form) {
/* @var FormInterface $form */
$propertyPath = $form->getPropertyPath();
$config = $form->getConfig();
foreach ($iterator as $form) {
/* @var FormInterface $form */
$propertyPath = $form->getPropertyPath();
$config = $form->getConfig();

if (null !== $propertyPath && $config->getMapped()) {
$form->setData($propertyPath->getValue($data));
}
if (null !== $propertyPath && $config->getMapped()) {
$form->setData($propertyPath->getValue($data));
}
}
}
Expand All @@ -48,6 +50,14 @@ public function mapDataToForms($data, array $forms)
*/
public function mapFormsToData(array $forms, &$data)
{
if (null === $data) {
return;
}

if (!is_array($data) && !is_object($data)) {
throw new UnexpectedTypeException($data, 'object, array or empty');
}

$iterator = new VirtualFormAwareIterator($forms);
$iterator = new \RecursiveIteratorIterator($iterator);

Expand Down
28 changes: 16 additions & 12 deletions src/Symfony/Component/Form/Form.php
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Form\Exception\AlreadyBoundException;
use Symfony\Component\Form\Exception\UnexpectedTypeException;
use Symfony\Component\Form\Exception\TransformationFailedException;
use Symfony\Component\Form\Util\FormUtil;
use Symfony\Component\Form\Util\PropertyPath;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
Expand Down Expand Up @@ -352,7 +353,7 @@ public function setData($modelData)
$viewData = $this->normToView($normData);

// Validate if view data matches data class (unless empty)
if ('' !== $viewData && null !== $viewData) {
if (!FormUtil::isEmpty($viewData)) {
$dataClass = $this->config->getDataClass();

$actualType = is_object($viewData) ? 'an instance of class ' . get_class($viewData) : ' a(n) ' . gettype($viewData);
Expand Down Expand Up @@ -492,12 +493,12 @@ public function bind($submittedData)
// since forms without children may also be compound.
// (think of empty collection forms)
if ($this->config->getCompound()) {
if (null === $submittedData || '' === $submittedData) {
$submittedData = array();
}

if (!is_array($submittedData)) {
throw new UnexpectedTypeException($submittedData, 'array');
if (!FormUtil::isEmpty($submittedData)) {
throw new UnexpectedTypeException($submittedData, 'array');
}

$submittedData = array();
}

foreach ($this->children as $name => $child) {
Expand All @@ -520,7 +521,7 @@ public function bind($submittedData)
$viewData = $this->getViewData();
}

if (null === $viewData || '' === $viewData) {
if (FormUtil::isEmpty($viewData)) {
$emptyData = $this->config->getEmptyData();

if ($emptyData instanceof \Closure) {
Expand All @@ -532,7 +533,7 @@ public function bind($submittedData)
}

// Merge form data from children into existing view data
if ($this->config->getCompound() && null !== $viewData) {
if ($this->config->getCompound()) {
$this->config->getDataMapper()->mapFormsToData($this->children, $viewData);
}

Expand Down Expand Up @@ -701,7 +702,7 @@ public function isEmpty()
}
}

return array() === $this->modelData || null === $this->modelData || '' === $this->modelData;
return FormUtil::isEmpty($this->modelData) || array() === $this->modelData;
}

/**
Expand Down Expand Up @@ -1048,9 +1049,12 @@ private function normToModel($value)
*/
private function normToView($value)
{
if (!$this->config->getViewTransformers()) {
// Scalar values should always be converted to strings to
// facilitate differentiation between empty ("") and zero (0).
// Scalar values should be converted to strings to
// facilitate differentiation between empty ("") and zero (0).
// Only do this for simple forms, as the resulting value in
// compound forms is passed to the data mapper and thus should
// not be converted to a string before.
if (!$this->config->getViewTransformers() && !$this->config->getCompound()) {
return null === $value || is_scalar($value) ? (string) $value : $value;
}

Expand Down
19 changes: 19 additions & 0 deletions src/Symfony/Component/Form/Util/FormUtil.php
Expand Up @@ -219,4 +219,23 @@ static public function isChoiceSelected($choice, $value)

return $choice === $value;
}

/**
* Returns whether the given data is empty.
*
* This logic is reused multiple times throughout the processing of
* a form and needs to be consistent. PHP's keyword `empty` cannot
* be used as it also considers 0 and "0" to be empty.
*
* @param mixed $data
*
* @return Boolean
*/
static public function isEmpty($data)
{
// Should not do a check for array() === $data!!!
// This method is used in occurrences where arrays are
// not considered to be empty, ever.
return null === $data || '' === $data;
}
}

0 comments on commit 5fe3f39

Please sign in to comment.