merged branch bschussek/issue3635 (PR #3820)

Commits
-------

65aa387 [Form] Fixed index generation in EntityChoiceList if ID is not an integer

Discussion
----------

[Form] Fixed index generation in EntityChoiceList if ID is not an integer

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3635
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3635)

src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php

@@ -49,13 +49,25 @@ class EntityChoiceList extends ObjectChoiceList
     private $entityLoader;
 
     /**
-     * The fields of which the identifier of the underlying class consists
-     *
-     * This property should only be accessed through identifier.
+     * The identifier field, if the identifier is not composite
      *
      * @var array
      */
-    private $identifier = array();
+    private $idField = null;
+
+    /**
+     * Whether to use the identifier for index generation
+     *
+     * @var Boolean
+     */
+    private $idAsIndex = false;
+
+    /**
+     * Whether to use the identifier for value generation
+     *
+     * @var Boolean
+     */
+    private $idAsValue = false;
 
     /**
      * Whether the entities have already been loaded.
@@ -82,9 +94,19 @@ class EntityChoiceList extends ObjectChoiceList
         $this->entityLoader = $entityLoader;
         $this->classMetadata = $manager->getClassMetadata($class);
         $this->class = $this->classMetadata->getName();
-        $this->identifier = $this->classMetadata->getIdentifierFieldNames();
         $this->loaded = is_array($entities) || $entities instanceof \Traversable;
 
+        $identifier = $this->classMetadata->getIdentifierFieldNames();
+
+        if (1 === count($identifier)) {
+            $this->idField = $identifier[0];
+            $this->idAsValue = true;
+
+            if ('integer' === $this->classMetadata->getTypeOfField($this->idField)) {
+                $this->idAsIndex = true;
+            }
+        }
+
         if (!$this->loaded) {
             // Make sure the constraints of the parent constructor are
             // fulfilled
@@ -174,12 +196,12 @@ class EntityChoiceList extends ObjectChoiceList
         if (!$this->loaded) {
             // Optimize performance in case we have an entity loader and
             // a single-field identifier
-            if (count($this->identifier) === 1 && $this->entityLoader) {
+            if ($this->idAsValue && $this->entityLoader) {
                 if (empty($values)) {
                     return array();
                 }
 
-                return $this->entityLoader->getEntitiesByIds(current($this->identifier), $values);
+                return $this->entityLoader->getEntitiesByIds($this->idField, $values);
             }
 
             $this->load();
@@ -204,7 +226,7 @@ class EntityChoiceList extends ObjectChoiceList
             // know that the IDs are used as values
 
             // Attention: This optimization does not check choices for existence
-            if (count($this->identifier) === 1) {
+            if ($this->idAsValue) {
                 $values = array();
 
                 foreach ($entities as $entity) {
@@ -239,7 +261,7 @@ class EntityChoiceList extends ObjectChoiceList
             // know that the IDs are used as indices
 
             // Attention: This optimization does not check choices for existence
-            if (count($this->identifier) === 1) {
+            if ($this->idAsIndex) {
                 $indices = array();
 
                 foreach ($entities as $entity) {
@@ -270,11 +292,10 @@ class EntityChoiceList extends ObjectChoiceList
     public function getIndicesForValues(array $values)
     {
         if (!$this->loaded) {
-            // Optimize performance for single-field identifiers. We already
-            // know that the IDs are used as indices and values
+            // Optimize performance for single-field identifiers.
 
             // Attention: This optimization does not check values for existence
-            if (count($this->identifier) === 1) {
+            if ($this->idAsIndex && $this->idAsValue) {
                 return $this->fixIndices($values);
             }
 
@@ -298,7 +319,7 @@ class EntityChoiceList extends ObjectChoiceList
      */
     protected function createIndex($entity)
     {
-        if (count($this->identifier) === 1) {
+        if ($this->idAsIndex) {
             return current($this->getIdentifierValues($entity));
         }
 
@@ -318,7 +339,7 @@ class EntityChoiceList extends ObjectChoiceList
      */
     protected function createValue($entity)
     {
-        if (count($this->identifier) === 1) {
+        if ($this->idAsValue) {
             return (string) current($this->getIdentifierValues($entity));
         }
 

src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/EntityChoiceListTest.php

@@ -15,9 +15,11 @@ use Symfony\Bridge\Doctrine\Form\ChoiceList\ORMQueryBuilderLoader;
 use Symfony\Bridge\Doctrine\Tests\DoctrineOrmTestCase;
 use Symfony\Bridge\Doctrine\Tests\Fixtures\ItemGroupEntity;
 use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIdentEntity;
+use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdentEntity;
 use Symfony\Bridge\Doctrine\Tests\Fixtures\NoToStringSingleIdentEntity;
 use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;
 use Symfony\Component\Form\Extension\Core\View\ChoiceView;
+use Doctrine\ORM\Tools\SchemaTool;
 
 class EntityChoiceListTest extends DoctrineOrmTestCase
 {
@@ -25,6 +27,8 @@ class EntityChoiceListTest extends DoctrineOrmTestCase
 
     const SINGLE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIdentEntity';
 
+    const SINGLE_STRING_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleStringIdentEntity';
+
     const COMPOSITE_IDENT_CLASS = 'Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeIdentEntity';
 
     private $em;
@@ -38,6 +42,24 @@ class EntityChoiceListTest extends DoctrineOrmTestCase
         parent::setUp();
 
         $this->em = $this->createTestEntityManager();
+
+        $schemaTool = new SchemaTool($this->em);
+        $classes = array(
+            $this->em->getClassMetadata(self::ITEM_GROUP_CLASS),
+            $this->em->getClassMetadata(self::SINGLE_IDENT_CLASS),
+            $this->em->getClassMetadata(self::SINGLE_STRING_IDENT_CLASS),
+            $this->em->getClassMetadata(self::COMPOSITE_IDENT_CLASS),
+        );
+
+        try {
+            $schemaTool->dropSchema($classes);
+        } catch(\Exception $e) {
+        }
+
+        try {
+            $schemaTool->createSchema($classes);
+        } catch(\Exception $e) {
+        }
     }
 
     protected function tearDown()
@@ -266,4 +288,24 @@ class EntityChoiceListTest extends DoctrineOrmTestCase
 
         $this->assertEquals(array(), $choiceList->getChoicesForValues(array()));
     }
+
+    // https://github.com/symfony/symfony/issues/3635
+    public function testSingleNonIntIdFallsBackToGeneration()
+    {
+        $entity1 = new SingleStringIdentEntity('Id 1', 'Foo');
+        $entity2 = new SingleStringIdentEntity('Id 2', 'Bar');
+
+        // Persist for managed state
+        $this->em->persist($entity1);
+        $this->em->persist($entity2);
+        $this->em->flush();
+
+        $choiceList = new EntityChoiceList(
+            $this->em,
+            self::SINGLE_STRING_IDENT_CLASS,
+            'name'
+        );
+
+        $this->assertSame(array(0 => $entity1, 1 => $entity2), $choiceList->getChoices());
+    }
 }