diff --git a/src/Symfony/Component/Form/CollectionField.php b/src/Symfony/Component/Form/CollectionField.php index 92cc3987e8..117fadc24f 100644 --- a/src/Symfony/Component/Form/CollectionField.php +++ b/src/Symfony/Component/Form/CollectionField.php @@ -77,6 +77,10 @@ class CollectionField extends Form public function setData($collection) { + if (null === $collection) { + $collection = array(); + } + if (!is_array($collection) && !$collection instanceof \Traversable) { throw new UnexpectedTypeException($collection, 'array or \Traversable'); } diff --git a/src/Symfony/Component/Form/EntityChoiceField.php b/src/Symfony/Component/Form/EntityChoiceField.php index e60af98ddf..5898ccb0a7 100644 --- a/src/Symfony/Component/Form/EntityChoiceField.php +++ b/src/Symfony/Component/Form/EntityChoiceField.php @@ -449,7 +449,7 @@ class EntityChoiceField extends ChoiceField } if (count($notFound) > 0) { - throw new TransformationFailedException('The entities with keys "%s" could not be found', implode('", "', $notFound)); + throw new TransformationFailedException(sprintf('The entities with keys "%s" could not be found', implode('", "', $notFound))); } return $result; @@ -466,10 +466,10 @@ class EntityChoiceField extends ChoiceField protected function transform($collectionOrEntity) { if (null === $collectionOrEntity) { - return $this->getOption('multiple') ? array() : ''; + return $this->isField() ? '' : array(); } - if (count($this->identifier) > 1) { + if (count($this->getIdentifierFields()) > 1) { // load all choices $availableEntities = $this->getEntities(); @@ -496,7 +496,6 @@ class EntityChoiceField extends ChoiceField } } - return parent::transform($result); } } \ No newline at end of file diff --git a/src/Symfony/Component/Form/Field.php b/src/Symfony/Component/Form/Field.php index fbb3097c21..617e7537ce 100644 --- a/src/Symfony/Component/Form/Field.php +++ b/src/Symfony/Component/Form/Field.php @@ -91,8 +91,7 @@ class Field extends Configurable implements FieldInterface $this->setNormalizationTransformer($this->getOption('normalization_transformer')); } - $this->normalizedData = $this->normalize($this->data); - $this->transformedData = $this->transform($this->normalizedData); + $this->setData($this->data); if (!$this->getOption('data')) { $this->setPropertyPath($this->getOption('property_path')); @@ -285,9 +284,15 @@ class Field extends Configurable implements FieldInterface */ public function setData($data) { + // All four transformation methods must be executed to make sure + // that all three data representations are synchronized + // Store data in between steps because processData() might use + // this data $this->data = $data; $this->normalizedData = $this->normalize($data); - $this->transformedData = $this->transform($this->normalizedData); + $this->transformedData = $this->transform($this->normalize($data)); + $this->normalizedData = $this->processData($this->reverseTransform($this->transformedData)); + $this->data = $this->denormalize($this->normalizedData); } /** diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index d8a6e38174..4a2972db6d 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -352,16 +352,6 @@ class Form extends Field implements \IteratorAggregate, FormInterface */ public function setData($data) { - if (empty($data)) { - if ($this->dataConstructor) { - $constructor = $this->dataConstructor; - $data = $constructor(); - } else if ($this->dataClass) { - $class = $this->dataClass; - $data = new $class(); - } - } - parent::setData($data); // get transformed data and pass its values to child fields @@ -380,6 +370,30 @@ class Form extends Field implements \IteratorAggregate, FormInterface } } + /** + * {@inheritDoc} + */ + protected function transform($value) + { + if (null === $this->getValueTransformer()) { + // Empty values must be converted to objects or arrays so that + // they can be read by PropertyPath in the child fields + if (empty($value)) { + if ($this->dataConstructor) { + $constructor = $this->dataConstructor; + return $constructor(); + } else if ($this->dataClass) { + $class = $this->dataClass; + return new $class(); + } else { + return array(); + } + } + } + + return parent::transform($value); + } + /** * Returns the data of the field as it is displayed to the user. * diff --git a/src/Symfony/Component/Form/HybridField.php b/src/Symfony/Component/Form/HybridField.php index 7c097c06b2..2aab06202b 100644 --- a/src/Symfony/Component/Form/HybridField.php +++ b/src/Symfony/Component/Form/HybridField.php @@ -135,4 +135,28 @@ class HybridField extends Form return Field::isEmpty(); } } + + /** + * {@inheritDoc} + */ + protected function transform($value) + { + if ($this->mode === self::FORM) { + return parent::transform($value); + } else { + return Field::transform($value); + } + } + + /** + * {@inheritDoc} + */ + protected function reverseTransform($value) + { + if ($this->mode === self::FORM) { + return parent::reverseTransform($value); + } else { + return Field::reverseTransform($value); + } + } } \ No newline at end of file diff --git a/src/Symfony/Component/Form/PropertyPath.php b/src/Symfony/Component/Form/PropertyPath.php index 5eebc349f3..f1979fd1aa 100644 --- a/src/Symfony/Component/Form/PropertyPath.php +++ b/src/Symfony/Component/Form/PropertyPath.php @@ -14,6 +14,7 @@ namespace Symfony\Component\Form; use Symfony\Component\Form\Exception\InvalidPropertyPathException; use Symfony\Component\Form\Exception\InvalidPropertyException; use Symfony\Component\Form\Exception\PropertyAccessDeniedException; +use Symfony\Component\Form\Exception\UnexpectedTypeException; /** * Allows easy traversing of a property path @@ -228,6 +229,10 @@ class PropertyPath implements \IteratorAggregate */ protected function readPropertyPath(&$objectOrArray, $currentIndex) { + if (!is_object($objectOrArray) && !is_array($objectOrArray)) { + throw new UnexpectedTypeException($objectOrArray, 'object or array'); + } + $property = $this->elements[$currentIndex]; if (is_object($objectOrArray)) { @@ -261,6 +266,10 @@ class PropertyPath implements \IteratorAggregate */ protected function writePropertyPath(&$objectOrArray, $currentIndex, $value) { + if (!is_object($objectOrArray) && !is_array($objectOrArray)) { + throw new UnexpectedTypeException($objectOrArray, 'object or array'); + } + $property = $this->elements[$currentIndex]; if ($currentIndex + 1 < $this->length) { diff --git a/tests/Symfony/Tests/Component/Form/EntityChoiceFieldTest.php b/tests/Symfony/Tests/Component/Form/EntityChoiceFieldTest.php index 21faab8f5d..6dfdc3f130 100644 --- a/tests/Symfony/Tests/Component/Form/EntityChoiceFieldTest.php +++ b/tests/Symfony/Tests/Component/Form/EntityChoiceFieldTest.php @@ -162,23 +162,53 @@ class EntityChoiceFieldTest extends DoctrineOrmTestCase $this->assertEquals('', $field->getDisplayedData()); } - public function testSetDataMultiple_null() + public function testSetDataMultipleExpanded_null() { $field = new EntityChoiceField('name', array( 'multiple' => true, + 'expanded' => true, 'em' => $this->em, 'class' => self::SINGLE_IDENT_CLASS, )); $field->setData(null); + $this->assertEquals(new ArrayCollection(), $field->getData()); + $this->assertEquals(array(), $field->getDisplayedData()); + } + + public function testSetDataMultipleNonExpanded_null() + { + $field = new EntityChoiceField('name', array( + 'multiple' => true, + 'expanded' => false, + 'em' => $this->em, + 'class' => self::SINGLE_IDENT_CLASS, + )); + $field->setData(null); + + $this->assertEquals(new ArrayCollection(), $field->getData()); + $this->assertEquals('', $field->getDisplayedData()); + } + + public function testSubmitSingleExpanded_null() + { + $field = new EntityChoiceField('name', array( + 'multiple' => false, + 'expanded' => true, + 'em' => $this->em, + 'class' => self::SINGLE_IDENT_CLASS, + )); + $field->submit(null); + $this->assertEquals(null, $field->getData()); $this->assertEquals(array(), $field->getDisplayedData()); } - public function testSubmitSingle_null() + public function testSubmitSingleNonExpanded_null() { $field = new EntityChoiceField('name', array( 'multiple' => false, + 'expanded' => false, 'em' => $this->em, 'class' => self::SINGLE_IDENT_CLASS, )); @@ -344,7 +374,7 @@ class EntityChoiceFieldTest extends DoctrineOrmTestCase 'property' => 'name', )); - $existing = new ArrayCollection(array($entity2)); + $existing = new ArrayCollection(array(0 => $entity2)); $field->setData($existing); $field->submit(array('0', '2')); diff --git a/tests/Symfony/Tests/Component/Form/FieldTest.php b/tests/Symfony/Tests/Component/Form/FieldTest.php index 95f3c519de..140405d447 100644 --- a/tests/Symfony/Tests/Component/Form/FieldTest.php +++ b/tests/Symfony/Tests/Component/Form/FieldTest.php @@ -15,6 +15,7 @@ require_once __DIR__ . '/Fixtures/Author.php'; require_once __DIR__ . '/Fixtures/TestField.php'; require_once __DIR__ . '/Fixtures/InvalidField.php'; require_once __DIR__ . '/Fixtures/RequiredOptionsField.php'; +require_once __DIR__ . '/Fixtures/FixedValueTransformer.php'; use Symfony\Component\Form\ValueTransformer\ValueTransformerInterface; use Symfony\Component\Form\PropertyPath; @@ -25,6 +26,7 @@ use Symfony\Tests\Component\Form\Fixtures\Author; use Symfony\Tests\Component\Form\Fixtures\TestField; use Symfony\Tests\Component\Form\Fixtures\InvalidField; use Symfony\Tests\Component\Form\Fixtures\RequiredOptionsField; +use Symfony\Tests\Component\Form\Fixtures\FixedValueTransformer; class FieldTest extends \PHPUnit_Framework_TestCase { @@ -224,7 +226,11 @@ class FieldTest extends \PHPUnit_Framework_TestCase { $this->field->setData(123); - $this->assertSame(123, $this->field->getData()); + // The values are synchronized + // Without value transformer, the field can't know that the data + // should be casted to an integer when the field is bound + // Even without binding, the data will thus be a string + $this->assertSame('123', $this->field->getData()); $this->assertSame('123', $this->field->getDisplayedData()); } @@ -349,21 +355,15 @@ class FieldTest extends \PHPUnit_Framework_TestCase public function testValuesAreTransformedCorrectly() { - // The value is first passed to the normalization transformer... - $normTransformer = $this->createMockTransformer(); - $normTransformer->expects($this->exactly(2)) - ->method('transform') - // Impossible to test with PHPUnit because called twice - // ->with($this->identicalTo(0)) - ->will($this->returnValue('norm[0]')); + $normTransformer = new FixedValueTransformer(array( + null => '', + 0 => 'norm[0]', + )); - // ...and then to the value transformer - $valueTransformer = $this->createMockTransformer(); - $valueTransformer->expects($this->exactly(2)) - ->method('transform') - // Impossible to test with PHPUnit because called twice - // ->with($this->identicalTo('norm[0]')) - ->will($this->returnValue('transform[norm[0]]')); + $valueTransformer = new FixedValueTransformer(array( + '' => '', + 'norm[0]' => 'transform[norm[0]]', + )); $field = new TestField('title', array( 'value_transformer' => $valueTransformer, @@ -379,18 +379,10 @@ class FieldTest extends \PHPUnit_Framework_TestCase public function testSubmittedValuesAreTrimmedBeforeTransforming() { - // The value is passed to the value transformer - $transformer = $this->createMockTransformer(); - $transformer->expects($this->once()) - ->method('reverseTransform') - ->with($this->identicalTo('a')) - ->will($this->returnValue('reverse[a]')); - - $transformer->expects($this->exactly(2)) - ->method('transform') - // Impossible to test with PHPUnit because called twice - // ->with($this->identicalTo('reverse[a]')) - ->will($this->returnValue('a')); + $transformer = new FixedValueTransformer(array( + null => '', + 'reverse[a]' => 'a', + )); $field = new TestField('title', array( 'value_transformer' => $transformer, @@ -404,18 +396,10 @@ class FieldTest extends \PHPUnit_Framework_TestCase public function testSubmittedValuesAreNotTrimmedBeforeTransformingIfDisabled() { - // The value is passed to the value transformer - $transformer = $this->createMockTransformer(); - $transformer->expects($this->once()) - ->method('reverseTransform') - ->with($this->identicalTo(' a ')) - ->will($this->returnValue('reverse[ a ]')); - - $transformer->expects($this->exactly(2)) - ->method('transform') - // Impossible to test with PHPUnit because called twice - // ->with($this->identicalTo('reverse[ a ]')) - ->will($this->returnValue(' a ')); + $transformer = new FixedValueTransformer(array( + null => '', + 'reverse[ a ]' => ' a ', + )); $field = new TestField('title', array( 'trim' => false, @@ -428,21 +412,6 @@ class FieldTest extends \PHPUnit_Framework_TestCase $this->assertEquals('reverse[ a ]', $field->getData()); } - /* - * This is important so that submit() can work even if setData() was not called - * before - */ - public function testWritePropertyTreatsEmptyValuesAsArrays() - { - $array = null; - - $field = new TestField('firstName'); - $field->submit('Bernhard'); - $field->writeProperty($array); - - $this->assertEquals(array('firstName' => 'Bernhard'), $array); - } - public function testWritePropertyDoesNotWritePropertyIfPropertyPathIsEmpty() { $object = new Author(); @@ -470,15 +439,16 @@ class FieldTest extends \PHPUnit_Framework_TestCase { // The value is passed to the value transformer $transformer = $this->createMockTransformer(); - $transformer->expects($this->once()) - ->method('reverseTransform') - ->will($this->throwException(new TransformationFailedException())); $field = new TestField('title', array( 'trim' => false, 'value_transformer' => $transformer, )); + $transformer->expects($this->once()) + ->method('reverseTransform') + ->will($this->throwException(new TransformationFailedException())); + $field->submit('a'); $this->assertEquals('a', $field->getDisplayedData()); diff --git a/tests/Symfony/Tests/Component/Form/Fixtures/FixedValueTransformer.php b/tests/Symfony/Tests/Component/Form/Fixtures/FixedValueTransformer.php new file mode 100644 index 0000000000..1084e1d3f7 --- /dev/null +++ b/tests/Symfony/Tests/Component/Form/Fixtures/FixedValueTransformer.php @@ -0,0 +1,35 @@ +mapping = $mapping; + } + + public function transform($value) + { + if (!array_key_exists($value, $this->mapping)) { + throw new \RuntimeException(sprintf('No mapping for value "%s"', $value)); + } + + return $this->mapping[$value]; + } + + public function reverseTransform($value) + { + $result = array_search($value, $this->mapping, true); + + if ($result === false) { + throw new \RuntimeException(sprintf('No reverse mapping for value "%s"', $value)); + } + + return $result; + } +} \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/Form/FormTest.php b/tests/Symfony/Tests/Component/Form/FormTest.php index 1a614069f3..d95bed9b5f 100644 --- a/tests/Symfony/Tests/Component/Form/FormTest.php +++ b/tests/Symfony/Tests/Component/Form/FormTest.php @@ -840,7 +840,7 @@ class FormTest extends \PHPUnit_Framework_TestCase */ public function testAddThrowsExceptionIfStringButNoFieldFactory() { - $form = new Form('author', array('data_class' => 'Application\Entity')); + $form = new Form('author'); $form->add('firstName'); } @@ -991,14 +991,6 @@ class FormTest extends \PHPUnit_Framework_TestCase $form->setData(new Author()); } - public function testSetDataToNull() - { - $form = new Form('author'); - $form->setData(null); - - $this->assertNull($form->getData()); - } - public function testSetDataToNullCreatesObjectIfClassAvailable() { $form = new Form('author', array( @@ -1022,6 +1014,18 @@ class FormTest extends \PHPUnit_Framework_TestCase $this->assertSame($author, $form->getData()); } + /* + * We need something to write the field values into + */ + public function testSetDataToNullCreatesArrayIfNoDataClassOrConstructor() + { + $author = new Author(); + $form = new Form('author'); + $form->setData(null); + + $this->assertSame(array(), $form->getData()); + } + public function testSubmitUpdatesTransformedDataFromAllFields() { $originalAuthor = new Author(); diff --git a/tests/Symfony/Tests/Component/Form/PropertyPathTest.php b/tests/Symfony/Tests/Component/Form/PropertyPathTest.php index eb949f9d0f..f687a2d5e5 100644 --- a/tests/Symfony/Tests/Component/Form/PropertyPathTest.php +++ b/tests/Symfony/Tests/Component/Form/PropertyPathTest.php @@ -192,6 +192,33 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase $path->getValue(new Author()); } + public function testGetValueThrowsExceptionIfNotObjectOrArray() + { + $path = new PropertyPath('foobar'); + + $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException'); + + $path->getValue('baz'); + } + + public function testGetValueThrowsExceptionIfNull() + { + $path = new PropertyPath('foobar'); + + $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException'); + + $path->getValue(null); + } + + public function testGetValueThrowsExceptionIfEmpty() + { + $path = new PropertyPath('foobar'); + + $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException'); + + $path->getValue(''); + } + public function testSetValueUpdatesArrays() { $array = array(); @@ -292,6 +319,36 @@ class PropertyPathTest extends \PHPUnit_Framework_TestCase $path->setValue(new Author(), 'foobar'); } + public function testSetValueThrowsExceptionIfNotObjectOrArray() + { + $path = new PropertyPath('foobar'); + $value = 'baz'; + + $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException'); + + $path->setValue($value, 'bam'); + } + + public function testSetValueThrowsExceptionIfNull() + { + $path = new PropertyPath('foobar'); + $value = null; + + $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException'); + + $path->setValue($value, 'bam'); + } + + public function testSetValueThrowsExceptionIfEmpty() + { + $path = new PropertyPath('foobar'); + $value = ''; + + $this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException'); + + $path->setValue($value, 'bam'); + } + public function testToString() { $path = new PropertyPath('reference.traversable[index].property');