bug #17006 [Form] Fix casting regression in DoctrineChoiceLoader (bendavies)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Fix casting regression in DoctrineChoiceLoader

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

In symfony 2.7, the [DoctrineChoiceLoader](9543b36a34/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php) and [IdReader](9543b36a34/src/Symfony/Bridge/Doctrine/Form/ChoiceList/IdReader.php) were introduce to replace the deprecated [EntityChoiceList](9543b36a34/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php)

There appears to have been a change in behaviour in the refactor, as the old `EntityChoiceList` [casts ID to strings](9543b36a34/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php (L248)), whereas the new `DoctrineChoiceLoader` [does not](https://github.com/symfony/symfony/blob/2.7/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php#L159). The casting behavior was [maintained elsewhere](https://github.com/symfony/symfony/blob/2.7/src/Symfony/Bridge/Doctrine/Form/ChoiceList/DoctrineChoiceLoader.php#L122), however.

Since the new `DoctrineChoiceLoader` deprecated `EntityChoiceList`, i'm calling this a regression.

Commits
-------

54bbade [Form] cast IDs to match deprecated behaviour of EntityChoiceList
This commit is contained in:
Tobias Schultze 2015-12-15 20:21:59 +01:00
commit 2d48af7745
3 changed files with 194 additions and 1 deletions

View File

@ -156,7 +156,7 @@ class DoctrineChoiceLoader implements ChoiceLoaderInterface
// "INDEX BY" clause to the Doctrine query in the loader,
// but I'm not sure whether that's doable in a generic fashion.
foreach ($unorderedObjects as $object) {
$objectsById[$this->idReader->getIdValue($object)] = $object;
$objectsById[(string) $this->idReader->getIdValue($object)] = $object;
}
foreach ($values as $i => $id) {

View File

@ -0,0 +1,57 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Bridge\Doctrine\Tests\Fixtures;
use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\GeneratedValue;
use Doctrine\ORM\Mapping\Id;
/** @Entity */
class SingleStringCastableIdEntity
{
/**
* @Id
* @Column(type="string")
* @GeneratedValue(strategy="NONE")
*/
protected $id;
/** @Column(type="string", nullable=true) */
public $name;
public function __construct($id, $name)
{
$this->id = new StringCastableObjectIdentity($id);
$this->name = $name;
}
public function __toString()
{
return (string) $this->name;
}
}
class StringCastableObjectIdentity
{
protected $id;
public function __construct($id)
{
$this->id = $id;
}
public function __toString()
{
return (string) $this->id;
}
}

View File

@ -24,6 +24,7 @@ use Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeIntIdEntity;
use Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeStringIdEntity;
use Symfony\Bridge\Doctrine\Tests\Fixtures\GroupableEntity;
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdEntity;
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringCastableIdEntity;
use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity;
use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView;
use Symfony\Component\Form\ChoiceList\View\ChoiceView;
@ -40,6 +41,7 @@ class EntityTypeTest extends TypeTestCase
const SINGLE_IDENT_NO_TO_STRING_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity';
const SINGLE_STRING_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdEntity';
const SINGLE_ASSOC_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity';
const SINGLE_STRING_CASTABLE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringCastableIdEntity';
const COMPOSITE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeIntIdEntity';
const COMPOSITE_STRING_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeStringIdEntity';
@ -67,6 +69,7 @@ class EntityTypeTest extends TypeTestCase
$this->em->getClassMetadata(self::SINGLE_IDENT_NO_TO_STRING_CLASS),
$this->em->getClassMetadata(self::SINGLE_STRING_IDENT_CLASS),
$this->em->getClassMetadata(self::SINGLE_ASSOC_IDENT_CLASS),
$this->em->getClassMetadata(self::SINGLE_STRING_CASTABLE_IDENT_CLASS),
$this->em->getClassMetadata(self::COMPOSITE_IDENT_CLASS),
$this->em->getClassMetadata(self::COMPOSITE_STRING_IDENT_CLASS),
);
@ -580,6 +583,139 @@ class EntityTypeTest extends TypeTestCase
$this->assertFalse($field['2']->getData());
}
public function testSubmitSingleNonExpandedStringCastableIdentifier()
{
$entity1 = new SingleStringCastableIdEntity(1, 'Foo');
$entity2 = new SingleStringCastableIdEntity(2, 'Bar');
$this->persist(array($entity1, $entity2));
$field = $this->factory->createNamed('name', 'entity', null, array(
'multiple' => false,
'expanded' => false,
'em' => 'default',
'class' => self::SINGLE_STRING_CASTABLE_IDENT_CLASS,
'choice_label' => 'name',
));
$field->submit('2');
$this->assertTrue($field->isSynchronized());
$this->assertSame($entity2, $field->getData());
$this->assertSame('2', $field->getViewData());
}
public function testSubmitSingleStringCastableIdentifierExpanded()
{
$entity1 = new SingleStringCastableIdEntity(1, 'Foo');
$entity2 = new SingleStringCastableIdEntity(2, 'Bar');
$this->persist(array($entity1, $entity2));
$field = $this->factory->createNamed('name', 'entity', null, array(
'multiple' => false,
'expanded' => true,
'em' => 'default',
'class' => self::SINGLE_STRING_CASTABLE_IDENT_CLASS,
'choice_label' => 'name',
));
$field->submit('2');
$this->assertTrue($field->isSynchronized());
$this->assertSame($entity2, $field->getData());
$this->assertFalse($field['0']->getData());
$this->assertTrue($field['1']->getData());
$this->assertNull($field['0']->getViewData());
$this->assertSame('2', $field['1']->getViewData());
}
public function testSubmitMultipleNonExpandedStringCastableIdentifierForExistingData()
{
$entity1 = new SingleStringCastableIdEntity(1, 'Foo');
$entity2 = new SingleStringCastableIdEntity(2, 'Bar');
$entity3 = new SingleStringCastableIdEntity(3, 'Baz');
$this->persist(array($entity1, $entity2, $entity3));
$field = $this->factory->createNamed('name', 'entity', null, array(
'multiple' => true,
'expanded' => false,
'em' => 'default',
'class' => self::SINGLE_STRING_CASTABLE_IDENT_CLASS,
'choice_label' => 'name',
));
$existing = new ArrayCollection(array(0 => $entity2));
$field->setData($existing);
$field->submit(array('1', '3'));
// entry with index 0 ($entity2) was replaced
$expected = new ArrayCollection(array(0 => $entity1, 1 => $entity3));
$this->assertTrue($field->isSynchronized());
$this->assertEquals($expected, $field->getData());
// same object still, useful if it is a PersistentCollection
$this->assertSame($existing, $field->getData());
$this->assertSame(array('1', '3'), $field->getViewData());
}
public function testSubmitMultipleNonExpandedStringCastableIdentifier()
{
$entity1 = new SingleStringCastableIdEntity(1, 'Foo');
$entity2 = new SingleStringCastableIdEntity(2, 'Bar');
$entity3 = new SingleStringCastableIdEntity(3, 'Baz');
$this->persist(array($entity1, $entity2, $entity3));
$field = $this->factory->createNamed('name', 'entity', null, array(
'multiple' => true,
'expanded' => false,
'em' => 'default',
'class' => self::SINGLE_STRING_CASTABLE_IDENT_CLASS,
'choice_label' => 'name',
));
$field->submit(array('1', '3'));
$expected = new ArrayCollection(array($entity1, $entity3));
$this->assertTrue($field->isSynchronized());
$this->assertEquals($expected, $field->getData());
$this->assertSame(array('1', '3'), $field->getViewData());
}
public function testSubmitMultipleStringCastableIdentifierExpanded()
{
$entity1 = new SingleStringCastableIdEntity(1, 'Foo');
$entity2 = new SingleStringCastableIdEntity(2, 'Bar');
$entity3 = new SingleStringCastableIdEntity(3, 'Bar');
$this->persist(array($entity1, $entity2, $entity3));
$field = $this->factory->createNamed('name', 'entity', null, array(
'multiple' => true,
'expanded' => true,
'em' => 'default',
'class' => self::SINGLE_STRING_CASTABLE_IDENT_CLASS,
'choice_label' => 'name',
));
$field->submit(array('1', '3'));
$expected = new ArrayCollection(array($entity1, $entity3));
$this->assertTrue($field->isSynchronized());
$this->assertEquals($expected, $field->getData());
$this->assertTrue($field['0']->getData());
$this->assertFalse($field['1']->getData());
$this->assertTrue($field['2']->getData());
$this->assertSame('1', $field['0']->getViewData());
$this->assertNull($field['1']->getViewData());
$this->assertSame('3', $field['2']->getViewData());
}
public function testOverrideChoices()
{
$entity1 = new SingleIntIdEntity(1, 'Foo');