[DoctrineBridge][Form] Fix EntityChoiceList when indexing by primary foreign key

giosh94mhz · giosh94mhz · commit fe4246aaa0fb · 2015-07-09T14:38:28.000+02:00
diff --git a/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php b/src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php
@@ -40,6 +40,13 @@ class EntityChoiceList extends ObjectChoiceList
      */
     private $classMetadata;
 
+    /**
+     * Metadata for target class of primary key association.
+     *
+     * @var ClassMetadata
+     */
+    private $idClassMetadata;
+
     /**
      * Contains the query builder that builds the query for fetching the
      * entities.
@@ -107,16 +114,21 @@ public function __construct(ObjectManager $manager, $class, $labelPath = null, E
         $this->class = $this->classMetadata->getName();
         $this->loaded = is_array($entities) || $entities instanceof \Traversable;
         $this->preferredEntities = $preferredEntities;
+        list(
+            $this->idAsIndex,
+            $this->idAsValue,
+            $this->idField
+        ) = $this->getIdentifierInfoForClass($this->classMetadata);
+
+        if (null !== $this->idField && $this->classMetadata->hasAssociation($this->idField)) {
+            $this->idClassMetadata = $this->em->getClassMetadata(
+                $this->classMetadata->getAssociationTargetClass($this->idField)
+            );
 
-        $identifier = $this->classMetadata->getIdentifierFieldNames();
-
-        if (1 === count($identifier)) {
-            $this->idField = $identifier[0];
-            $this->idAsValue = true;
-
-            if (in_array($this->classMetadata->getTypeOfField($this->idField), array('integer', 'smallint', 'bigint'))) {
-                $this->idAsIndex = true;
-            }
+            list(
+                $this->idAsIndex,
+                $this->idAsValue
+            ) = $this->getIdentifierInfoForClass($this->idClassMetadata);
         }
 
         if (!$this->loaded) {
@@ -228,7 +240,7 @@ public function getChoicesForValues(array $values)
                 // "INDEX BY" clause to the Doctrine query in the loader,
                 // but I'm not sure whether that's doable in a generic fashion.
                 foreach ($unorderedEntities as $entity) {
-                    $value = $this->fixValue(current($this->getIdentifierValues($entity)));
+                    $value = $this->fixValue($this->getSingleIdentifierValue($entity));
                     $entitiesByValue[$value] = $entity;
                 }
 
@@ -274,7 +286,7 @@ public function getValuesForChoices(array $entities)
                 foreach ($entities as $i => $entity) {
                     if ($entity instanceof $this->class) {
                         // Make sure to convert to the right format
-                        $values[$i] = $this->fixValue(current($this->getIdentifierValues($entity)));
+                        $values[$i] = $this->fixValue($this->getSingleIdentifierValue($entity));
                     }
                 }
 
@@ -314,7 +326,7 @@ public function getIndicesForChoices(array $entities)
                 foreach ($entities as $i => $entity) {
                     if ($entity instanceof $this->class) {
                         // Make sure to convert to the right format
-                        $indices[$i] = $this->fixIndex(current($this->getIdentifierValues($entity)));
+                        $indices[$i] = $this->fixIndex($this->getSingleIdentifierValue($entity));
                     }
                 }
 
@@ -372,7 +384,7 @@ public function getIndicesForValues(array $values)
     protected function createIndex($entity)
     {
         if ($this->idAsIndex) {
-            return $this->fixIndex(current($this->getIdentifierValues($entity)));
+            return $this->fixIndex($this->getSingleIdentifierValue($entity));
         }
 
         return parent::createIndex($entity);
@@ -392,7 +404,7 @@ protected function createIndex($entity)
     protected function createValue($entity)
     {
         if ($this->idAsValue) {
-            return (string) current($this->getIdentifierValues($entity));
+            return (string) $this->getSingleIdentifierValue($entity);
         }
 
         return parent::createValue($entity);
@@ -415,6 +427,36 @@ protected function fixIndex($index)
         return $index;
     }
 
+    /**
+     * Get identifier information for a class.
+     *
+     * @param ClassMetadata $classMetadata The entity metadata
+     *
+     * @return array Return an array with idAsIndex, idAsValue and identifier
+     */
+    private function getIdentifierInfoForClass(ClassMetadata $classMetadata)
+    {
+        $identifier = null;
+        $idAsIndex = false;
+        $idAsValue = false;
+
+        $identifiers = $classMetadata->getIdentifierFieldNames();
+
+        if (1 === count($identifiers)) {
+            $identifier = $identifiers[0];
+
+            if (!$classMetadata->hasAssociation($identifier)) {
+                $idAsValue = true;
+
+                if (in_array($classMetadata->getTypeOfField($identifier), array('integer', 'smallint', 'bigint'))) {
+                    $idAsIndex = true;
+                }
+            }
+        }
+
+        return array($idAsIndex, $idAsValue, $identifier);
+    }
+
     /**
      * Loads the list with entities.
      *
@@ -438,6 +480,33 @@ private function load()
         $this->loaded = true;
     }
 
+    /**
+     * Returns the first (and only) value of the identifier fields of an entity.
+     *
+     * Doctrine must know about this entity, that is, the entity must already
+     * be persisted or added to the identity map before. Otherwise an
+     * exception is thrown.
+     *
+     * @param object $entity The entity for which to get the identifier
+     *
+     * @return array The identifier values
+     *
+     * @throws RuntimeException If the entity does not exist in Doctrine's identity map
+     */
+    private function getSingleIdentifierValue($entity)
+    {
+        $value = current($this->getIdentifierValues($entity));
+
+        if ($this->idClassMetadata) {
+            $class = $this->idClassMetadata->getName();
+            if ($value instanceof $class) {
+                $value = current($this->idClassMetadata->getIdentifierValues($value));
+            }
+        }
+
+        return $value;
+    }
+
     /**
      * Returns the values of the identifier fields of an entity.
      *
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php b/src/Symfony/Bridge/Doctrine/Tests/Fixtures/SingleAssociationToIntIdEntity.php
@@ -0,0 +1,38 @@
+<?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\Id;
+use Doctrine\ORM\Mapping\Column;
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\OneToOne;
+
+/** @Entity */
+class SingleAssociationToIntIdEntity
+{
+    /** @Id @OneToOne(targetEntity="SingleIntIdNoToStringEntity", cascade={"ALL"}) */
+    protected $entity;
+
+    /** @Column(type="string", nullable=true) */
+    public $name;
+
+    public function __construct(SingleIntIdNoToStringEntity $entity, $name)
+    {
+        $this->entity = $entity;
+        $this->name = $name;
+    }
+
+    public function __toString()
+    {
+        return (string) $this->name;
+    }
+}
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListSingleAssociationToIntIdTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListSingleAssociationToIntIdTest.php
@@ -0,0 +1,86 @@
+<?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\Form\ChoiceList;
+
+use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity;
+use Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity;
+use Symfony\Bridge\Doctrine\Form\ChoiceList\EntityChoiceList;
+
+/**
+ * Test choices generated from an entity with a primary foreign key.
+ *
+ * @author Premi Giorgio <giosh94mhz@gmail.com>
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+abstract class AbstractEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListTest
+{
+    protected function getEntityClass()
+    {
+        return 'Symfony\Bridge\Doctrine\Tests\Fixtures\SingleAssociationToIntIdEntity';
+    }
+
+    protected function getClassesMetadata()
+    {
+        return array(
+            $this->em->getClassMetadata($this->getEntityClass()),
+            $this->em->getClassMetadata('Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity'),
+        );
+    }
+
+    protected function createChoiceList()
+    {
+        return new EntityChoiceList($this->em, $this->getEntityClass(), 'name');
+    }
+
+    /**
+     * @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
+     */
+    protected function createObjects()
+    {
+        $innerA = new SingleIntIdNoToStringEntity(-10, 'inner_A');
+        $innerB = new SingleIntIdNoToStringEntity(10, 'inner_B');
+        $innerC = new SingleIntIdNoToStringEntity(20, 'inner_C');
+        $innerD = new SingleIntIdNoToStringEntity(30, 'inner_D');
+
+        $this->em->persist($innerA);
+        $this->em->persist($innerB);
+        $this->em->persist($innerC);
+        $this->em->persist($innerD);
+
+        return array(
+            new SingleAssociationToIntIdEntity($innerA, 'A'),
+            new SingleAssociationToIntIdEntity($innerB, 'B'),
+            new SingleAssociationToIntIdEntity($innerC, 'C'),
+            new SingleAssociationToIntIdEntity($innerD, 'D'),
+        );
+    }
+
+    protected function getChoices()
+    {
+        return array('_10' => $this->obj1, 10 => $this->obj2, 20 => $this->obj3, 30 => $this->obj4);
+    }
+
+    protected function getLabels()
+    {
+        return array('_10' => 'A', 10 => 'B', 20 => 'C', 30 => 'D');
+    }
+
+    protected function getValues()
+    {
+        return array('_10' => '-10', 10 => '10', 20 => '20', 30 => '30');
+    }
+
+    protected function getIndices()
+    {
+        return array('_10', 10, 20, 30);
+    }
+}
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/AbstractEntityChoiceListTest.php
@@ -39,7 +39,7 @@ protected function setUp()
         $this->em = DoctrineTestHelper::createTestEntityManager();
 
         $schemaTool = new SchemaTool($this->em);
-        $classes = array($this->em->getClassMetadata($this->getEntityClass()));
+        $classes = $this->getClassesMetadata();
 
         try {
             $schemaTool->dropSchema($classes);
@@ -73,6 +73,11 @@ abstract protected function getEntityClass();
 
     abstract protected function createObjects();
 
+    protected function getClassesMetadata()
+    {
+        return array($this->em->getClassMetadata($this->getEntityClass()));
+    }
+
     /**
      * @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
      */
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/LoadedEntityChoiceListSingleAssociationToIntIdTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/LoadedEntityChoiceListSingleAssociationToIntIdTest.php
@@ -0,0 +1,32 @@
+<?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\Form\ChoiceList;
+
+/**
+ * @author Premi Giorgio <giosh94mhz@gmail.com>
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+class LoadedEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListSingleAssociationToIntIdTest
+{
+    /**
+     * @return \Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceListInterface
+     */
+    protected function createChoiceList()
+    {
+        $list = parent::createChoiceList();
+
+        // load list
+        $list->getChoices();
+
+        return $list;
+    }
+}
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdTest.php
@@ -0,0 +1,32 @@
+<?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\Form\ChoiceList;
+
+/**
+ * @author Premi Giorgio <giosh94mhz@gmail.com>
+ * @author Bernhard Schussek <bschussek@gmail.com>
+ */
+class UnloadedEntityChoiceListSingleAssociationToIntIdTest extends AbstractEntityChoiceListSingleAssociationToIntIdTest
+{
+    public function testGetIndicesForValuesIgnoresNonExistingValues()
+    {
+        $this->markTestSkipped('Non-existing values are not detected for unloaded choice lists.');
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testLegacyGetIndicesForValuesIgnoresNonExistingValues()
+    {
+        $this->markTestSkipped('Non-existing values are not detected for unloaded choice lists.');
+    }
+}
diff --git a/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdWithQueryBuilderTest.php b/src/Symfony/Bridge/Doctrine/Tests/Form/ChoiceList/UnloadedEntityChoiceListSingleAssociationToIntIdWithQueryBuilderTest.php
